Create a safe nullpointer in regards to memory leaks

Question

Look at this:

    char (*options)[MAXLEN];
    char *ptr;
    ptr = strtok(input, " \r\n");
    if(!ptr)
        continue;
    int i = 0;
    optionen = malloc(sizeof(*options));
    if(!options)
        die("malloc");
    while(ptr){
        if(i > 0){
            options = realloc(options, (i+1)*sizeof(*options));
            if(!options)
                die("malloc");
        }
        strcpy(options[i], ptr);
        if(!options[i])
            die("strcpy");
        ptr = strtok(NULL, " \r\n");
        i++;
    }


/*create another entry options[i] that is a nullpointer*/

Purpose is that the exec(3) command requires a nullpointer as the last entry to the *options[] array to work properly.

Problem: How can I add another entry to the array that is a NULL - pointer? I understand that I cannot allocate another options[i] and set it to NULL because some guy on stackoverflow told me to never do that (memory leak).

Note: input is some array that contains some commandline - input(char input[MAXLEN]; ) and die() just calls perror() and then exit()

Answer 1

Whoever told you that you "cannot allocate another options[i] and set it to NULL" was wrong. That is exactly what you do.

However, you have bugs in your code, and the bugs suggest that you don't understand what it means to "allocate another options[i]."

char (*options)[MAXLEN];  /* This is wrong */
char *ptr;
ptr = strtok(input, " \r\n");
if(!ptr)
    continue;
int i = 0;
optionen = malloc(sizeof(*options));
if(!options)
    die("malloc");
while(ptr){
    if(i > 0){
        options = realloc(options, (i+1)*sizeof(*options));
        if(!options)
            die("malloc");
    }
    strcpy(options[i], ptr); /* This is also wrong */
    if(!options[i])
        die("strcpy");
    ptr = strtok(NULL, " \r\n");
    i++;
}

First off, char (*options)[MAXLEN] is the wrong kind of array, and an entry in it cannot be set to NULL. You need instead char **options.

Second off, realloc operation that you are already doing is, in fact, allocating more options[i] slots. But you also need to allocate space for the strings themselves (but not for the NULL). You do that with strdup.

options[i] = strdup(ptr); /* Instead of the "also wrong" line */

(Depending on what input is, you might be able to get away with just using the string pointers strtok returns, but without seeing more of your code I can't be sure that's safe.)

And then, after the loop, you simply set the final options[i] slot to NULL and you're done.

I would structure the loop a bit differently, like this:

char **options;
char *ptr;
size_t i, asize;

ptr = strtok(input, " \t\r\n");
if (!ptr) continue;

options = 0;
i = 0;
asize = 2;
do {
    while (i >= asize) {
        asize *= 2;
        options = xreallocarray(options, asize, sizeof(char *));
    }
    options[i++] = xstrdup(ptr);
    ptr = strtok(0, " \t\r\n");
} while (ptr);

while (i >= asize) {
    asize *= 2;
    options = xreallocarray(options, asize, sizeof(char *));
}
options[i] = 0;

execve(options[0], options, environ);
die("execve");

The functions xreallocarray and xstrdup are nonstandard, but they should be in your bag of simple functions that you add to every program that you write. Here are their definitions. They use the die() function you already have.

void *
xreallocarray(void *optr, size_t nmemb, size_t size)
{
    /* s1*s2 <= SIZE_MAX if both s1 < K and s2 < K where K = sqrt(SIZE_MAX+1) */
    const size_t MUL_NO_OVERFLOW = ((size_t)1) << (sizeof(size_t) * 4);

    if ((nmemb >= MUL_NO_OVERFLOW || size >= MUL_NO_OVERFLOW) &&
        nmemb > 0 && SIZE_MAX / nmemb < size) {
        errno = ENOMEM;
        die("malloc");
    }

    void *rv = realloc(optr, size * nmemb);
    if (!rv)
        die("malloc");
    return rv;
}

char *
xstrdup(const char *s)
{
    size_t n = strlen(s) + 1;
    char *rv = malloc(n);
    if (!rv)
        die("malloc");
    memcpy(rv, s, n);
    return rv;
}

Answer 2

My suggestion: Create another array as follows and use it in the call to exec.

char** args = malloc((i+1)*sizeof(*args));
for (int j = 0; j < i; ++j )
{
   args[j] = options[j];
}
args[i] = NULL;

execvp(..., args);