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?

Improve this question

5 Answers 5

Reset to default
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);

}
Improve this answer
Sign up to request clarification or add additional context in comments.

10 Comments

The compiler doesn't seem to know what getline is, is this because of a missing #include line or just the compiler itself?
getline is a GNU function in C: gnu.org/s/libc/manual/html_node/Line-Input.html
What compiler and what Operating System?
i686-apple-darwin9-gcc-4.0.1 (OS X 10.5.8)
It says: Undefined symbols: "_getline", referenced from: _main in ccvpqdih.o _main in ccvpqdih.o _main in ccvpqdih.o ld: symbol(s) not found collect2: ld returned 1 exit status
|
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).

Improve this answer

6 Comments

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 ';'"
@Nick Double check your code; I wrote a little test program and it compiled fine for me.
Woops, I forgot a semicolon on the MAX_LEN = 512 line. Fixed now.
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.
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).
|
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.

Improve this answer

1 Comment

And another one for argv[0] which is the name of the command (normally).
1

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

Improve this answer

2 Comments

So how exactly would I do this?
Look up malloc/free and make sure each pointer in "cmd" is allocated enough space.
-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.

Improve this answer

1 Comment

The '%as' specification is not standard C behaviour - useful though it may be.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.