Linux shell assignment. Execvp failing to execute commands

Question

I am trying to create a simple shell using C for linux. I cannot figure out why execvp() keeps failing. I know execvp doesn't require a path to be passed with the arguments. I tried following someone's recommendation by running the char array of my commands through strtok.

I keep getting execvp has failed: no such file or directory. I am just passing a simple "ls" as my command to test.

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

#define MAXARGS         20
#define CMDPROMPT       "theshell>"
#define ARGPROMPT       "Next argument: "
#define prompt(n)       printf("%s", (n) == 0 ? CMDPROMPT : ARGPROMPT);

int main(int argc, char* argv[])
{
        char arg[MAXARGS] = {0};
        int i = 0,
            should_run = 1, pid, exitstatus;

        while (should_run)
        {
                prompt(*arg);

                fgets(arg, MAXARGS, stdin);                                                                                                                 
                                                                                                                                                            
                char *arg = strtok(arg, " ");                                                                                                               
                                                                                                                                                            
                char *arg_tok[MAXARGS];                                                                                                                     
                                                                                                                                                            
                while(arg)                                                                                                                                  
                {                                                                                                                                           
                        arg_tok[i++] = arg;                                                                                                                 
                        arg = strtok(NULL, " ");                                                                                                            
                }                                                                                                                                           
                                                                                                                                                            
                pid = fork();                                                                                                                               
                                                                                                                                                            
                switch(pid)
                {
                        case -1:
                                perror("Fork Failed\n");
                                exit(1);
                        case 0:
                                execvp(arg_tok[0], arg_tok);
                                perror("execvp has failed");
                                exit(1);
                        default:
                                while( wait(&exitstatus) != pid );
                                printf("Child process exited with status %d, %d\n", exitstatus>>8, exitstatus&0377);
                }
        }
}

When you exit your while() loop, you need arg_tok[i] = NULL; or initialize char *arg_tok[MAXARGS] = {NULL}; -- the next pointer after your last argument must be NULL. From man 3 execvp you see "The array of pointers must be terminated by a null pointer." — David C. Rankin
– David C. Rankin, Commented Mar 3, 2021 at 4:54
Tried both options, still getting same error message. If I pass the arg array instead of arg_tok, execvp seems to accept the command, but it doesn't execute and exits from the child process. I appreciate the response. — AKO
– AKO, Commented Mar 3, 2021 at 11:47
And char *arg = strtok(arg, " "); SHADOWS char arg[MAXARGS] = {0};. Always compile with warnings enabled and for gcc/cland add -Wshadow to catch shadowed variables. — David C. Rankin
– David C. Rankin, Commented Mar 3, 2021 at 21:21

David C. Rankin · Accepted Answer · 2021-03-03 22:40:12Z

You have a number of problems primarily detailed in the comments above. You don't terminate the execvp() list of pointers with a NULL and you shadow arg with arg declared as:

    char *arg = strtok(arg, " ");

and then redeclared as:

        char *arg = strtok(arg, " ");

And then lastly, you fail to remove the '\n' from the string pointed to by the last pointer. Recall, fgets() reads and includes the '\n' in the buffer filled. So simply change your delimiter list for strtok() to include '\n', e.g. " \n".

Putting it altogether, you can do:

#include <string.h>
#include <unistd.h>
#include <stdio.h>
#include <string.h>
#include <stdlib.h>
#include <sys/wait.h>                           /* include sys/wait.h for wait() */

#define MAXARGS         20
#define CMDPROMPT       "theshell> "            /* add space after > */
#define ARGPROMPT       "Next argument: "
#define prompt(n)       printf("%s", (n) == 0 ? CMDPROMPT : ARGPROMPT);

int main (void)
{
    char arg[MAXARGS] = "";
    int i = 0,
        should_run = 1, pid, exitstatus;

    while (should_run)
    {
        prompt (0);

        fgets (arg, MAXARGS, stdin);

        char *p = strtok (arg, " \n");           /* fix shadow of arg, included \n as delim */

        char *arg_tok[MAXARGS] = {NULL};

        while (p) {
            arg_tok[i++] = p;
            p = strtok(NULL, " \n");
        }

        pid = fork();

        switch(pid)
        {
            case -1:
                perror("Fork Failed\n");
                exit(1);
            case 0:
                execvp (arg_tok[0], arg_tok);
                perror ("execvp has failed");
                exit(1);
            default:
                while( wait(&exitstatus) != pid );
                printf ("Child process exited with status %d, %d\n",
                        exitstatus>>8, exitstatus&0377);
        }
    }
}

(note: the addition of the header sys/wait.h for wait())

Example Use/Output

$ ./bin/execvp_ls
theshell> ls -al fh
total 152
drwxr-xr-x  2 david david   4096 Jul  1  2018 .
drwxr-xr-x 32 david david 131072 Mar  3 15:16 ..
-rw-r--r--  1 david david    602 Jul  1  2018 Readme_ld_reqd_opts.txt
-rw-r--r--  1 david david    124 Jul  1  2018 conversion.c
-rw-r--r--  1 david david     30 Jul  1  2018 conversion.h
-rw-r--r--  1 david david    250 Jul  1  2018 convert_driver.c
Child process exited with status 0, 0
theshell> ^C

Note also, your use of macros, while clever, will usually lead to more problems than they solve. Simply writing the prompt to be displayed is a far more readable way to go. While the macro use here is trivial here, this only grows with code length and complexity.

Look things over and let me know if you have further questions.

Collectives™ on Stack Overflow

Linux shell assignment. Execvp failing to execute commands

1 Answer 1

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

Comments

Your Answer

Sign up or log in

Post as a guest

Related