Having trouble with execvp

Question

I am having some trouble with a basic shell program I am trying to write in c. Whenever I try calling exec in a function such as ls, execvp will return with an error stating that the file or directory could not be found. I think that the problem is with the parsing because in main, the command can be printed but in the function it is blank. Thanks

Here is the code for the function :

int extern_process(char *arg[]){
    pid_t pid;
    int errnum, ifFail;
    printf("i%si\n",arg[0]);
    pid = fork();

    if(pid == -1){
        errnum = errno;
        fprintf(stderr,"Error: fork %s", strerror(errnum));
        return FAIL;
    } else if(pid == 0){
        ifFail = execvp(arg[0],arg);
        if(ifFail < 0){
            errnum = errno;
            fprintf(stderr,"Error: exec %s", strerror(errnum));
            return FAIL;
        }
    } else {
        pid = wait(NULL);
    }
    return SUCCESS;
}

Here is the code for the parsing function just in case:

void parse_cmd(char *retval[], char *cmd){
    char *tmp;
    char a[100];
    strcpy(a,cmd);
    int i = 0;
    tmp = strtok(a," \n\t\0");

    if(retval == NULL){
        fprintf(stderr, "Error with allocation\n");
        return;
    }
    if(tmp == NULL){
        printf("Error with parsing.\n");
        return;
    }
    while(tmp != NULL){
        retval[i] = tmp;
        tmp = strtok(NULL," \n\t\0");
        i++;
    }
    retval[i] = NULL;
}

Here is the output:

shell> ls 
ls
i i 
Error: exec no file or directory found 

Answer 1

I'm pretty sure strtok returns a pointer that refers to that first argument which, in your case, is a stack allocation. Returning an array of pointers to that stack allocation would result in undefined behavior, I believe. This may or may not be the cause of your problem. It's difficult to know without seeing more of the code. To test, try changing this part of your code like this:

void parse_cmd(char *retval[], char *cmd){
    char *tmp;
    char *a = strdup(cmd);
    int i = 0;

Before using it in production, you need to work out some way to ensure that you free "a" or you'll get a leak. Maybe you could just return it instead of void and free it from elsewhere, or you could actually strdup() each token and write a function to free them all or whatever works for you.

If there are other problems, they may be in other code. I don't really see anything else wrong here.