Building a basic shell, more specifically using execvp()

Question

In my program I am taking user input and parsing it into a 2d char array. The array is declared as:

char parsedText[10][255] = {{""},{""},{""},{""},{""},
            {""},{""},{""},{""},{""}};

and I am using fgets to grab the user input and parsing it with sscanf. This all works as I think it should.

After this I want to pass parsedText into execvp, parsedText[0] should contain the path and if any arguments are supplied then they should be in parsedText[1] thru parsedText[10].

What is wrong with execvp(parsedText[0], parsedText[1])?

One thing probably worth mentioning is that if I only supply a command such as "ls" without any arguments it appears to work just fine.

Here is my code:

#include <stdio.h>
#include <string.h>
#include <unistd.h>
#include "308shell.h"

int main( int argc, char *argv[] )
{
char prompt[40] = "308sh";
char text[40] = "";
char parsedText[10][40] = {{""},{""},{""},{""},{""},
                           {""},{""},{""},{""},{""}};

// Check for arguments to change the prompt.
if(argc >= 3){
    if(!(strcmp(argv[1], "-p"))){
        strcpy(prompt, argv[2]);
    }
}

strcat(prompt, "> ");

while(1){
    // Display the prompt.
    fputs(prompt, stdout);
    fflush(stdout);

    // Grab user input and parse it into parsedText. 
    mygetline(text, sizeof text);
    parseInput(text, parsedText);

    // Check if the user wants to exit.
    if(!(strcmp(parsedText[0], "exit"))){
        break;
    }
    execvp(parsedText[0], parsedText[1]);
    printf("%s\n%s\n", parsedText[0], parsedText[1]);
}

return 0;
}

char *mygetline(char *line, int size)
{
if ( fgets(line, size, stdin) )
{
    char *newline = strchr(line, '\n'); /* check for trailing '\n' */
    if ( newline )
    {
        *newline =  '\0'; /* overwrite the '\n' with a terminating null */
    }
}

return line;
}

char *parseInput(char *text, char parsedText[][40]){
char *ptr = text;
char field [ 40 ];
int n;
int count = 0;

while (*ptr != '\0') {
    int items_read = sscanf(ptr, "%s%n", field, &n);
    strcpy(parsedText[count++], field);
    field[0]='\0';
    if (items_read == 1)
        ptr += n; /* advance the pointer by the number of characters read     */
    if ( *ptr != ' ' ) {
        strcpy(parsedText[count], field);
        break; /* didn't find an expected delimiter, done? */
    }
    ++ptr; /* skip the delimiter */
}

}

Answer 1

execvp takes a pointer to a pointer (char **), not a pointer to an array. It's supposed to be a pointer to the first element of an array of char * pointers, terminated by a null pointer.

Edit: Here's one (not very good) way to make an array of pointers suitable for execvp:

char argbuf[10][256] = {{0}};
char *args[10] = { argbuf[0], argbuf[1], argbuf[2], /* ... */ };

Of course in the real world your arguments probably come from a command line string the user entered, and they probably have at least one character (e.g. a space) between them, so a much better approach would be to either modify the original string in-place, or make a duplicate of it and then modify the duplicate, adding null terminators after each argument and setting up args[i] to point to the right offset into the string.

You could instead do a lot of dynamic allocation (malloc) every step of the way, but then you have to write code to handle every possible point of failure. :-)