-1

I'm trying to learn more about Unix and am writing a shell in C. My execute() command replaces the function system(), and it works for commands like ls, clear, etc. However, I can't get it work for the cat command. Rather, the number of error messages doubles every time (see below).

==>cat
==>a
==>error in child==>a
a 
a
==>error in child==>a
==>error in child==>a
a
a
==>error in child==>a
==>error in child==>a
==>error in child==>a
==>error in child==>a
a
a
==>error in child==>a
==>error in child==>a
==>error in child==>a
==>error in child==>a
==>error in child==>a
==>error in child==>a
==>error in child==>a
==>error in child==>a
a
a

This is the execute() command. If I use system() anywhere I use execute(), cat works great, but with execute(), it doesn't. In the case where the command inputted is just cat, then args[0] is cat and args[1] is the null terminator. The length is 1.

int execute (char** args)
{
        int status;
        pid_t pid;

        switch(pid = fork())
        {
                case -1:
                        status = -1;
                        fprintf(stderr, "failed to fork"); 
                        break;
                case 0:
                        execvp(args[0], args);
                        fprintf(stderr,"error in child");
                default:
                        if(!WIFSIGNALED(status) && !WIFEXITED(status))
                              waitpid(pid, &status, WUNTRACED);
        }
        return 0;
}

This is how execute() is used in the main() method. The command from the keyboard is stored as a string in cmndbuf (which is declared as char cmndbuf[MAX_BUFFER]). cmndbuf is then split up and put into the new array called splits.

if (cmndbuf[0]){
    char** splits=malloc(sizeof(char*)*MAX_BUFFER + 1);
    int i;
    char* to_sep=cmndbuf;
    for(i=0; i<MAX_ARGS; i++)
    {
        splits[i] = strsep(&to_sep, " ");
        if(splits[i]==NULL) break;
    }
    execute(splits);
    //system(cmndbuf); //this works
    free(splits);                                     
}

Thanks guys! :)

quil
  • 417
  • 1
  • 6
  • 16
  • How is `execute` called? Are you sure that the `args` array is `NULL` terminated? Notice that you don't use `length`. Why do you pass it? – Basile Starynkevitch Nov 05 '17 at 17:56
  • BTW, did you read documentation of [cat(1)](http://man7.org/linux/man-pages/man1/cat.1.html)? Without any additional argument it is useless. And we don't know anything about your file descriptors. Consider using [strace(1)](http://man7.org/linux/man-pages/man1/strace.1.html) with `-f` to debug your shell – Basile Starynkevitch Nov 05 '17 at 17:58
  • 1
    A "C shell" is a particular family of shells -- it has a specific meaning, different from "shell written in C". – Charles Duffy Nov 05 '17 at 18:01
  • 2
    ...more immediately to the point, though, what you've provided is less than a full reproducer. One can't take your code *as it is, without adding anything else* and reproduce the problem, or definitively tell whether a fix addresses it. See the documentation on building a [mcve]. – Charles Duffy Nov 05 '17 at 18:02
  • @BasileStarynkevitch This is a minimal example; I'd forgotten to remove length in it. I also know that cat alone is useless, as it just echoes back what you've written. Again, I'm trying to do a minimal example. (If I can get cat to echo back what I'm writing, that'll get other things working.) I'll add a bit more information on how execute() is called. – quil Nov 05 '17 at 18:13
  • @CharlesDuffy Thanks for the clarification; the title has been edited and a bit more information added. – quil Nov 05 '17 at 18:22

1 Answers1

0

The lines

if(!WIFSIGNALED(status) && !WIFEXITED(status))
    waitpid(pid, &status, WUNTRACED);

won't work because waitpid() needs to executed at least once, and then it needs to be executed continually until the condition becomes false. That is, a do-while loop is needed:

do {
  wpid = waitpid(pid, &status, WUNTRACED);
} while (!WIFEXITED(status) && !WIFSIGNALED(status));
quil
  • 417
  • 1
  • 6
  • 16