execvp() - unsupported SysV option

Question

I am trying to write a simple shell in C which takes in a command and uses a child process to execute that command. For example, if I input:

ps -ael

my child process should execute that command along with its arguments. I print out my commands as they are stored in an array. This what I see:

Array[0] = ps
Array[1] = -ael  
Array[2] = NULL

When I execute I get this:

error: unsupported SysV option

Usage:
 ps [options]

 Try 'ps --help <simple|list|output|threads|misc|all>'
  or 'ps --help <s|l|o|t|m|a>'
 for additional help text.

For more details see ps(1).

My code below.

int main(void)
{
    char *args[MAX_LINE/2 +1]; // command line arguments
    char *cmdLine;
    int should_run = 1; // flag to determine when to exit the program
    int i, x;


    printf("osh> ");
    fflush(stdout);
    fgets(cmdLine, MAX_LINE, stdin);


    char *token = strtok(cmdLine, " ");
    int position = 0;
    while (token != NULL)
    {
        args[position++] = token;
        token = strtok(NULL, " ");
    }
    i = 0;
    while (args[i] != NULL)
    {
        printf("Array[%d] = %s\n", i, args[i]);
        i++;
    }
    if (args[i] == NULL) printf("Array[%d] = NULL", i);
    x = 0;
    pid_t pid;

    /* fork a child process*/
    pid = fork();

    if (pid < 0)
    {
        /*Error occured*/
        fprintf(stderr, "Fork failed.");
        return 1;
    }
    else if (pid == 0)
    {
        /*child process*/

        execvp(args[0], args); //error here
    }
    else
    {
        /*Parent process*/

        wait(NULL);

        printf("\nChild complete\n");
    }

}

run it with `strace -f -eexecve your-prog` to see what it really happening. And check that no hidden whitespaces are contained (e.g. print `Array.. . = '%s'`) — ensc, Mar 08 '18 at 01:03
the `fgets()` function inputs everything, including the '\n' (or the buffer is almost full) then appends a NUL byte. The posted code needs to replace the '\n' with a NUL byte. Otherwise, the '\n' is part of the parameters placed in `args[]` this is why the error message is occurring. — user3629249, Mar 08 '18 at 18:03
regarding: `fgets(cmdLine, MAX_LINE, stdin);` and `char *cmdLine;` The first parameter of the `fgets()` function is expected to be a pointer to an array/buffer to receive the data read. The `cmdLine` is declared as an 'uninitialized pointer. I.E. that pointer contains what ever trash happens to be on the stack at the location of `cmdLine`. This results in undefined behavior and can lead to a seg fault event. Suggest: `char cmdLine[ MAX_LINE ];` — user3629249, Mar 08 '18 at 18:04
the posted code does not compile! Amongst other things, it is missing the needed `#include` statements and is missing a definition of `MAX_LINE` Do you expect us to read your mind as to which header file are included and what value you defined for `MAX_LINE`? — user3629249, Mar 08 '18 at 18:06
regarding: `while (args[i] != NULL)` How can the code expect any of the pointers that makeup `args[]` ever to be NULL when the code never set any of the pointers to NULL? Due to this missing statement, the result is undefined behavior and can lead to a seg fault event. — user3629249, Mar 08 '18 at 18:13
regarding: `execvp(args[0], args);` the `exec...()` functions never return unless it failed to create the sub process. So the statement should be immediately followed by: `perror( "execvp failed" ); exit( EXIT_FAILURE );` which will output why the call to `execvp()` failed (and the enclosed text) to `stderr`. — user3629249, Mar 08 '18 at 18:16
for ease of readability and understanding: follow the axiom: *only one statement per line and (at most) one variable declaration per statement.* — user3629249, Mar 08 '18 at 18:21
regarding: `fprintf(stderr, "Fork failed.");` it is best to output WHY the function call failed. Suggest replacing with: `perror( "fork failed' );` So both the enclosed text and the reason for the failure are output to `stderr` — user3629249, Mar 08 '18 at 18:24

Barmar · Accepted Answer · 2018-03-08T01:08:42.903

2

The string returned by fgets() includes the newline, but you're not removing it from the string. So you're setting args[1] to "-ael\n", and \n is not a valid option.

Include newline in your strtok() delimiters:

char *token = strtok(cmdLine, " \n");
int position = 0;
while (token != NULL)
{
    args[position++] = token;
    token = strtok(NULL, " \n");
}

and then it will not be included in the tokens.

You should have been able to see this in your output, I'll bet it printed:

Array[0] = ps
Array[1] = -ael  

Array[2] = NULL

with a blank line there.

BTW, I don't see where you're ever setting the last argument to NULL. The while loop stops when strtok() returns NULL, so it never assigns that result to args[position++]. You need to add:

args[position] = NULL;

after the loop.

And there's no need for if (args[i] == NULL) -- the loop before that stops when that condition is met, so it's guaranteed to be true.

edited Mar 08 '18 at 01:08

answered Mar 08 '18 at 01:03

Barmar

741,623
53
500
612

Thank you so much, this worked! I had a feeling it had something to do white space.. – Connor Orischak Mar 08 '18 at 01:06
this answer fails to mention the two instances of undefined behavior in the posted code. – user3629249 Mar 08 '18 at 18:27
@user3629249 I mentioned the missing null pointer, but I didn't examine all the code, I was just looking for the cause of the error message. – Barmar Mar 08 '18 at 18:39
The missing `#define` and `#include` are presumably just because he didn't copy them to the question. – Barmar Mar 08 '18 at 18:41
those questions on stackoverflow.com about a run time problem are 'off topic' when the posted code is not a [mcve]. Code that does not compile, due to missing parts, fails the requirements – user3629249 Mar 08 '18 at 19:12

execvp() - unsupported SysV option

1 Answers1