Trying to use execvp() in C with user input in unix

Question

I'm trying to make a program that will prompt the user for a command, then use exec to execute that command.

For instance if they gave me "ls -la" I would have to execute that command. I've tried the following code:

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

int main()
{

    int ret, num_args;

    printf("Enter number of arguments (Example: \"ls -la\" has 1 argument): ");
    scanf("%d", &num_args);

    char *cmd[num_args];

    printf("Enter command name: ");
    scanf("%s", &cmd[0]);

    int i;
    for (i = 0; i < num_args; i++)
    {
            printf("Enter parameter: ");
            scanf("%s", &cmd[i]);
    }

    execvp(cmd[0], cmd);
}

However, when I tried the following run it gave me a "segmentation fault"

$ ./a.out 
Enter number of arguments (Example: "ls -la" has 1 argument): 2
Enter command name: ls
Enter parameter: -la
Enter parameter: .
Segmentation fault
$

Any ideas?

Answer 1

If your implementation supports it you should use the safer getline() instead on scanf() or fgets(). getline() will safely handle long lines and NULL characters. It will allocate enough memory to fit the entire line. getline() can allocate memory so you will have to free it yourself later on.

Here is the glibc getline() documentation.

Here is a quick modification to use getline (It still needs work, error checking and I haven't fully checked it for correctness yet):

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

int main()
{

    printf("Enter number of arguments (Example: \"ls -la\" has 1 argument): \n");

    char *num = NULL;
    size_t sz = 0;
    getline(&num, &sz, stdin);

    int num_args;
    sscanf(num, "%d", &num_args);

    char *cmd[num_args+2];
    memset(cmd, 0, sizeof(char*) * (num_args+2));

    printf("Enter command name: \n");


    int len = getline(&cmd[0], &sz, stdin); 

    cmd[0][len-1] = '\0';

    int i;
    for (i = 1; i < num_args+1; i++)
    {
        printf("Enter parameter: \n");
        sz = 0;
        len = getline(&cmd[i], &sz, stdin);
        cmd[i][len-1] = '\0';
    }

    return execvp(cmd[0], cmd);

}

Answer 2

You need to allocate memory for your strings. The following line only allocates num_args worth of pointers to char:

char *cmd[num_args];

First of all, you'll be getting num_args + 1 strings (don't forget that the command itself is cmd[0]). The easiest way is to statically allocate the memory as an array of character buffers:

const unsigned int MAX_LEN = 512; // Arbitrary number
char cmd[num_args + 1][MAX_LEN];

However, now you can't use scanf to read in a line because the user could input a string that's longer than your character buffer. Instead, you'll have to use fgets, which can limit the number of characters the user can input:

fgets(cmd[i], MAX_LEN, stdin);

Keep in mind that fgets also reads newline characters, so make sure to strip any stray ones that show up (but don't assume that they're there).

Answer 3

Also, you need one more entry in the argv you pass to execvp, which has to be (char *)NULL to let it know that it's reached the end of the list.

Answer 4

You haven't actually allocated any memory for the strings pointed to by the cmd array.

Answer 5

Take a look at the man page for scanf(). One of the neatest things it can do is automatically allocate the string buffers on the fly, you need to supply a pointer to a string instead of just passing the string and supply the %as format.

char *my_string;
scanf("%as", &my_string);

Then you don't need to bother with preallocating, don't need to bother with buffer overflows, etc. Just remember to free() it after you're done with it.