Passing an array to execvp() from the user's input

Question

I'm trying to pass arguments entered by the user to execvp().

So far I've split up the string. If the user types ls -a, temp is saved as "ls" and "-a" followed by a NULL character. I'm not quite sure how to point to this properly in execvp. In examples I've seen it using execvp(temp[position], temp). I know the way I'm trying to do it at the moment is wrong, but I'm not sure how to do it properly! At the moment I'm getting a segmentation fault.

int main(int argc, char *argv[]) 
{
    char line[124];
    int size = 124;
    char *temp = NULL;

    while(fgets(line, size, stdin) != NULL ) {
        if (strcmp(line, "exit\n") == 0) {
            exit(EXIT_SUCCESS);
        }
        temp = strtok(line, " ");
        while (temp != NULL) {
            printf("%s\n", temp);
            temp = strtok(NULL, " ");
        }
        execvp(temp, &temp);    
    }
    return EXIT_SUCCESS;
}

Answer 1

Your problem is that temp is a single pointer, and you need to pass an array of pointers to execvp().

Something like:

    enum { MAX_ARGS = 64 };
    char *args[MAX_ARGS];
    char **next = args;

    temp = strtok(line, " ");
    while (temp != NULL)
    {
        *next++ = temp;
        printf("%s\n", temp);
        temp = strtok(NULL, " ");
    }
    *next = NULL;
    execvp(args[0], args);

Note that the argument list has been given a null pointer as a terminator, just like argv[argc] == NULL in main(). Clearly, I've skimped on the error checking (if you pass more than 63 arguments, you're going to overflow args array). But this contains the core idea.

---

With this example, I can't seem to get the simple command of ls to work, I've tried mkdir and echo and they seem to work fine. Passing ls returns -1 from execvp().

I'm not sure what the problem might be — all of these work for me:

• ls
• ls -l
• ls -l madump.c (where madump.c happens to be a file in the directory I'm testing in)

The code I used was:

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>

int main(void) 
{
    char line[1024];

    while (fgets(line, sizeof(line), stdin) != NULL)
    {
        if (strcmp(line, "exit\n") == 0)
            exit(EXIT_SUCCESS);

        char *args[64];
        char **next = args;
        char *temp = strtok(line, " \n");
        while (temp != NULL)
        {
            *next++ = temp;
            printf("%s\n", temp);
            temp = strtok(NULL, " \n");
        }
        *next = NULL;

        puts("Checking:");
        for (next = args; *next != 0; next++)
            puts(*next);

        execvp(args[0], args);
    }

    return EXIT_SUCCESS;
}

Note that I added \n to the strtok() token list, after creating a directory with a newline at the end of its name. Good for annoying friends and baffling semi-educated enemies, but a nuisance from most other perspectives. Note how I print out the data that's going to be passed to execvp() just before actually doing so. Often, I'd use printf("<<%s>>\n", *next); instead of just puts() to get an unambiguous indication of where the arguments start and end.

The output from running the command (doit) was:

$ ./doit
ls -l madump.c
ls
-l
madump.c
Checking:
ls
-l
madump.c
-rw-r--r--  1 jleffler  staff  2352 Jul 28  2011 madump.c
$

What did you get from your version?

Answer 2

As your code looks currently, after your while(temp != NULL) finishes, temp will be NULL!

execvp expects the first argument to be the path of the file which will be the new process image. The second argument is expected to be an array of NULL terminated strings where the last member of that array is a NULL pointer and the first member is the filename of the file specified in the first argument.

To implement this in your code, consider the following while loop instead:

char **argList = NULL;
unsigned int numArgs = 0;
while (temp != NULL) {
    numArgs++;

    /* Reallocate space for your argument list */
    argList = realloc(argList, numArgs * sizeof(*argList));

    /* Copy the current argument */
    argList[numArgs - 1] = malloc(strlen(temp) + 1, 1); /* The +1 for length is for the terminating '\0' character */
    snprintf(argList[numArgs - 1], strlen(temp) + 1, "%s", temp);

    printf("%s\n", temp);
    temp = strtok(NULL, " ");
}

/* Store the last NULL pointer */
numArgs++;
argList = realloc(argList, numArgs * sizeof(*argList));
argList[numArgs - 1] = NULL;

/* Finally, pass this to execvp */
execvp(argList[0], argList);
/* If you reach here, execvp() failed */

The code I provided above does not do some error-checking (such as when realloc or malloc fails), but basically keep these points in mind:

1. Argument 1: Path name of the file which is going to be put in memory.
2. Argument 2: A list of arguments where first member of that list = file name, last member = NULL pointer.

Please look at the documentation for better clarity and a very simple example.

Answer 3

I'm guessing the segfault is because you are passing a NULL pointer to execvp. The loop immediately above the call ensures this.

To do what you're trying to do, you'll need to create an array of string pointers for what you currently have called temp. This becomes the argv array for the program you call. That's why the command is usually used as execvp (temp[0], temp) - argv[0] is usually the name of the program.

So try creating an array of string pointers, and have each one point to a tokenized word from line. You may need to use malloc, although if you want to be clever you might be able to point directly into line. If you do that, you'll need to set the character immediately after every 'word' to \0.