Segfault on execvp using command line arguments

908 views Asked by At

I am working on a programming assignment that ask me to write a code able to read a command from the command line, together with its argument, and execute the program using the execvp. This is my code:

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

int main(int argc, const char * argv[]) {

    char **cmd;
    int i;

    if (argc == 1){

        fprintf(stdout, "No command inserted!\n");
        exit(EXIT_SUCCESS);

    }

    cmd = (char **) malloc( argc * sizeof(char *));
    cmd[0] = strdup(argv[1]);

    if (argc > 2){

    for (i = 1 ; i < argc - 1  ; i++ ){

            cmd[i] = (char *) malloc( strlen(argv[i+1]) * sizeof(char) );
        strcpy(cmd[i], argv[i+1]);

    }

    cmd[argc] = NULL;
    execvp(cmd[0], cmd);

    fprintf(stderr, "Failed Execution or not existing command!!\n");
    exit(EXIT_FAILURE);

    }

    cmd[1] = NULL;

    execvp(cmd[0], cmd);

    fprintf(stderr, "Failed Execution or not existing command!!\n");
    exit(EXIT_FAILURE);

    return 0;
}

The code works fine typing no arguments command such as:

./a.out who
./a.out ls

but result in a 'Segmentation Fault:11' when writing commands like:

./a.out ls -l
./a.out more file.txt

I can't figure out where is the problem...

2

There are 2 answers

0
Stephan Lechner On BEST ANSWER

There are at least two points where you exceed array bounds:

cmd[i] = (char *) malloc( strlen(argv[i+1]) * sizeof(char) )

is one off as it does not consider the terminating \0-character, such that a strcpy(cmd[i], argv[i+1]) then will exceed the bounds. Write ...

cmd[i] = (char *) malloc( (strlen(argv[i+1]) + 1) * sizeof(char) )

instead. BTW: sizeof(char) is always 1 by definition.

Further,

cmd = (char **) malloc( argc * sizeof(char *));
...
cmd[argc] = NULL;

is again one off. It should be cmd = (char **) malloc( (argc+1) * sizeof(char *)) when you like to assign cmd[argc] = NULL.

0
Andrew Puglionesi On

I had a similar issue. execvp() appeared to do nothing, but in fact the child process I ran it under was crashing with a segfault. The fix ended up being fixing char arrays I'd used to store different transformations of the input. I hadn't used malloc() at all for some of these arrays (I am not a C person and am not proud of that mistake), which only became an issue as the program grew more complex. For the general user brought here, I would advise checking carefully for unallocated pointers or, as above, for pointers with insufficient size. This happened to me even though the array passed to execvp() was properly allocated and had the needed contents (as determined through GDB) with the right types.