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);
}
}
}
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.