Storing as using result of strtok in array in C

Question

I'm trying to split the input from fgets using strtok, and store the results in an array, i.e. newArgs, so I can then call execvp and essentially execute the input passed by fgets.

E.g. ls -la will map to /bin/ls -la and execute correctly.

#include <sys/types.h>
#include <sys/stat.h>
#include <unistd.h>
#include <string.h>
#include <stdlib.h>
#include <stdio.h>

int main(int argc, char * argv[])
{
    char buff[1024];
    fgets(buff, 1024, stdin);
    buff[strcspn(buff, "\n")] = 0;
    printf("%s\n", buff);
    printf("%d\n", strlen(buff));
    char *newArgs[30];
    char *token;
    char delim[2] = " ";
    token = strtok(buff, delim);
    int i = 0;
    while(token != NULL) 
    {
        if(newArgs[i])
        {
            sprintf(newArgs[i], "%s", token);
            printf("%s\n", newArgs[i]); 
        }
        token = strtok(NULL, delim);
        i++;
    }
    execvp(newArgs[0], newArgs);

    return 0;
}

I keep getting a Segmentation fault, even though I'm checking the existence of newArgs[i], which is a little odd. Any ideas as to what's going wrong?

Answer 1

Problems I see:

1. You are using uninitialized values of newArgs[i]. You have:

   char *newArgs[30];
   

   This is an array of uninitialized pointers. Then, you go on to use them as:

   if(newArgs[i])
   

   That is cause for undefined behavior. You can fix that by initializing the pointers to NULL.

   char *newArgs[30] = {};
   

2. You haven't allocated memory for newArgs[i] before calling

   sprintf(newArgs[i], "%s", token);
   

   That is also cause for undefined behavior. You can fix that by using:

   newArgs[i] = strdup(token);
   

3. The list of arguments being passed to execvp must contains a NULL pointer.

   From http://linux.die.net/man/3/execvp (emphasis mine):

   The execv(), execvp(), and execvpe() functions provide an array of pointers to null-terminated strings that represent the argument list available to the new program. The first argument, by convention, should point to the filename associated with the file being executed. The array of pointers must be terminated by a NULL pointer.

   You are missing the last requirement. You need o make sure that one of the elements of newArgs is a NULL pointer. This problem will go away if you initialize the pointers to NULL.