3

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?

Kredns
  • 36,461
  • 52
  • 152
  • 203

5 Answers5

3

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).

Andrew Keeton
  • 22,195
  • 6
  • 45
  • 72
  • Trying it that way gave me a bunch of compiler errors. It's saying cmd[num_arghs + 1][MAX_LEN]; isn't right: "syntax error before ';'" –  Aug 24 '09 at 00:55
  • @Nick Double check your code; I wrote a little test program and it compiled fine for me. – Andrew Keeton Aug 24 '09 at 01:06
  • Woops, I forgot a semicolon on the `MAX_LEN = 512` line. Fixed now. – Andrew Keeton Aug 24 '09 at 01:08
  • `scanf` can also limit the number of characters the user can input - use `"%511s"` if your buffer is of size 512, as in your example. – caf Aug 24 '09 at 04:13
  • Oh, and if you do this you can't directly pass `cmd` to `execvp` anymore - it needs an array of pointers to char (terminated by a NULL entry), not an array of arrays of char (this is a good example of a case in which they're **not** equivalent). – caf Aug 24 '09 at 04:17
  • Note that you need num_args + 2 string pointers; one for the command name, num_args for the arguments, and one for the terminating null pointer. – Jonathan Leffler Aug 25 '09 at 06:07
3

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);

}
Karl Voigtland
  • 7,637
  • 34
  • 29
2

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.

hobbs
  • 223,387
  • 19
  • 210
  • 288
1

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

Paul Tomblin
  • 179,021
  • 58
  • 319
  • 408
-1

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.

KFro
  • 764
  • 3
  • 8