5

I have been working on a custom shell script and have come to a small error when redirecting output with the code given below. In its current state the code works perfectly but when passing to execvp args throws errors such as : (ls ">" no such file or directory). I know this is because it is passing the whole args[] to the parent shell which isn't working. Adding in the args[j] = NULL takes away the "<"/ ">" thus fixing the error, but also causes the redirections to not work anymore. How can I get it to not throw an error but also work properly? I have read multiple versions of this question but cant seem to find an answer. Thanks in advance for any help.

switch (fork()){
        case -1:
        fprintf(stderr, "error forking");

        case 0://CHILD

        for(int j = 0; j < size; j++){

            if(!strcmp(args[j], "<")){//looking for input character
            ++ext;
            if((in = open(args[j+1], O_RDONLY)) < 0){//open file for reading
                fprintf(stderr, "error opening file\n");
            }
            dup2(in, STDIN_FILENO);//duplicate stdin to input file
            close(in);//close after use
            //args[j] = NULL;
                }//end input chech


            if(!strcmp(args[j],">")){//looking for output character
            ++ext;
                out = creat(args[j+1], 0644);//create new output file           
            dup2(out, STDOUT_FILENO);//redirect stdout to file
            close(out);//close after usere  
        //  args[j] = NULL;
            }//end output check 

            if(!strcmp(args[j], ">>")){//looking for append
            ++ext;
            int append = open(args[j+1],O_CREAT | O_RDWR | O_APPEND, 0644);
                dup2(append, STDOUT_FILENO);
                close(append);
             // args[j] = NULL;
            }                

         }//end loop


        execvp(args[0],args);//execute in parent
        fprintf(stderr, "error in child execi \n");//error
        exit(0);    

         default://PARENT
        wait(&status);  //wait for child to finish  
    }//end switch
snipshow7
  • 87
  • 1
  • 8
  • Do you mean a 'shell program' (replacement for a standard shell such as Bash), or do you mean a 'shell script' (a program that is run by a particular shell)? Be careful; programs aren't necessarily scripts, and if you post C source, you're not posting a script — at least, using the conventional meanings. Also, be aware that there is a specific C shell, which has a different syntax from Bash and other shells derived from the Bourne shell. So you probably mean "Redirecting I/O in a custom shell program written in C" or thereabouts. – Jonathan Leffler Oct 23 '18 at 00:35

1 Answers1

5

When you are parsing redirections (e.g. <, >, >>) and doing your open/dup2, you have to strip them from the argument list you pass to execvp.

So, given your args, you need a second (e.g. args_clean) argument list that you only copy over the program name and its arguments.

And, you need an extra increment of j to skip over the redirection file in args (i.e. just doing j + 1 isn't equivalent).


Here's the cleaned up child code [please pardon the gratuitous style cleanup]:

char *args_clean[size];
int cleanidx = 0;

for (int j = 0; j < size; j++) {
    if (!strcmp(args[j], "<")) {        // looking for input character
        ++j;
        if ((in = open(args[j], O_RDONLY)) < 0) {   // open file for reading
            fprintf(stderr, "error opening file\n");
        }
        dup2(in, STDIN_FILENO);         // duplicate stdin to input file
        close(in);                      // close after use
        continue;
    }                                   // end input chech

    if (!strcmp(args[j], ">")) {        // looking for output character
        ++j;
        out = creat(args[j], 0644); // create new output file
        dup2(out, STDOUT_FILENO);       // redirect stdout to file
        close(out);                     // close after usere
        continue;
    }                                   // end output check

    if (!strcmp(args[j], ">>")) {       // looking for append
        ++j;
        int append = open(args[j], O_CREAT | O_RDWR | O_APPEND, 0644);

        dup2(append, STDOUT_FILENO);
        close(append);
        continue;
    }

    args_clean[cleanidx++] = args[j];
}                                       // end loop

args_clean[cleanidx] = NULL;
execvp(args_clean[0], args_clean);                  // execute in parent
fprintf(stderr, "error in child execi \n"); // error
exit(0);

Also, see my answer here for something similar with pipes: fd leak, custom Shell

And, for a full blown shell, see my answer: Implementing input/output redirection in a Linux shell using C and look at the embedded pastebin link

Craig Estey
  • 30,627
  • 4
  • 24
  • 48
  • Now that is self-promotion -- well done! (as well as a good answer...) – David C. Rankin Oct 23 '18 at 02:06
  • @DavidC.Rankin Thanks. Yes, shameless. But, see this SO answer [_not_ mine] https://stackoverflow.com/a/52442453/5382650 and my comment to Vittorio (i.e. I agreed with what he said, did +1 and told him so). Peter Schneider then chimed in. I [just] linked to an answer I did on codereview because all that I would have said to refute Peter in the comments was contained in the linked answer. Eh, more self promotion, but this one was standing up for principles :-) And, I _had_ to stand up for my principles on the codereview answer because got DV'ed to -3 until I did some clarification – Craig Estey Oct 23 '18 at 02:23
  • Actually, not _too_ many mods. I glanced over your `open/dup2/close` code but it seemed pretty good, so I didn't bother to check it rigorously, and I didn't change any of it. Many OP's have trouble with that, too [_particularly_ for pipes], so you're ahead of the game. – Craig Estey Oct 23 '18 at 02:58
  • Wow I would not have thought of doing it like this but it fixes everything, thank you so much for the help! – snipshow7 Oct 23 '18 at 03:04
  • The proof is in the pudding `:)` – David C. Rankin Oct 23 '18 at 03:05