1

Hopefully a simple question. I'm trying to learn, simultaneously, fork(), pipe(), and waitpid() and running into some problems.

if (pipe(myout)<0 || pipe(myin)<0 || pipe(myerr)<0) { perror("Couldn't make pipes"); return; }
int childpid=fork();
if (childpid==0) { //child
    fdopen(myout[1], "w");
    fdopen(myin[1], "r");
    fdopen(myerr[1], "w");
    dup2(myout[1],  1);
    dup2(myin[1], 0);
    dup2(myerr[1], 2);
    printf("This should be seen\n");
    fclose(stdout); fclose(stdin); fclose(stderr);
    sleep(10);
    _exit(0);
 } else { //parent, wait on child
    printf("parent, monitoring\n");
    sim_out=fdopen(myout[0], "r");
    sim_in=fdopen(myin[0], "w");
    sim_err=fdopen(myerr[0], "r");
    printf("have my fds\n");
    int status;
    do {
        int ch;
        if (read(myout[0], &ch, 1)>0)
            write(1, &ch, 1);
        else printf("no go\n");
            waitpid(childpid, &status, WNOHANG);
    } while (!WIFEXITED(status) && !WIFSIGNALED(status));
}

I'm getting:

parent, monitoring have my fds T

before the program exits - that is, the loop runs only once. I have a check below that, and it's coming up WIFEXITED() so the process is supposed to have exited normally. But what bothers me is that there's a sleep(10) before that happens, and this happens immediately - not to mention that the child processes are left running for the remaining wait time.

I'm fundamentally misunderstanding something, clearly. My expectation was that the parent loop would block until it sees a character from the child, read it, then check to see if it was still alive. I certainly didn't expect waitpid() to set WIFEXITED when the child was still alive.

Where am I wrong?

Robert
  • 6,412
  • 3
  • 24
  • 26
  • use the code button to format code, not html tags please. I've fixed it for you. – Evan Teran Dec 03 '10 at 17:37
  • The do loop executes once, outputting 'T'. What does WIFEXITED(status) or WIFSIGNALED(status) return if status hasn't been set (or is 0)? – Aaron H. Dec 03 '10 at 17:46

2 Answers2

5

I think I can see several issues. I'll try to mention them in order of appearance.

  • You should check fork for the return value -1 which indicates an error (no child will have been forked in this case).
  • All your calls to fdopen leak resources (depending on the implementation; the one on RHEL4 leaks). They return a FILE* which you could then use in fwrite etc. and then close by doing fclose on them. But you throw that value away. You don't need to open the pipe for reading/writing. Pipes are suitable for that when created.
  • The child should close the ends of the pipe it does not use. Add close (myin [1]); myin [1] = -1; close (myout [0]); myout [0] = -1; close (myerr [0]); myerr [0] = -1;
  • The dup2 are technically fine on all Linux variants I know, but it is customary to use the first fd of a pipe for reading and the other for writing. Thus, your dup2s would be best changed to dup2 (myin [0], STDIN_FILENO); dup2 (myout [1], STDOUT_FILENO); dup2 (myerr [1], STDERR_FILENO);
  • The parent should close the ends of the pipe it does not use. Add close (myin [0]); myin [0] = -1; close (myout [1]); myout [1] = -1; close (myerr [1]); myerr [1] = -1;
  • Your main problem: You check the possibly uninitialized status for your child's exit code. But waitpid has not yet been called. You should check waitpid's exit code and not evaluate status if it returns something other than childpid.

Edit

Since only the pipe ends that you actually need are open now, the operating system will detect a broken pipe for you. The pipe breaks when the child does fclose (stdout). The parent can still proceed to read all the data that may be in the pipe, but after that read will return zero, indicating a broken pipe.

Thus, you can actually spare the call to waitpid. You could instead simply wait for read to return zero. This is not 100% equivalent, though, since your child closes its end of the pipe before it goes to sleep (causing the parent to proceed when all data have been read), whereas the waitpid version, of course, only proceeds when the child actually died.

Community
  • 1
  • 1
dennycrane
  • 2,301
  • 18
  • 15
  • Very very helpful. I figured I was making some pretty boneheaded mistakes here, and this may answer another one of my questions too. – Robert Dec 03 '10 at 18:48
  • Detailed, helpful answer. Deserved +1 – salezica Dec 03 '10 at 20:18
  • This answered my other question as well. I was bolting this over a functioning command interpreter that I was reluctant to change (hence the fiddling with stdin/stdio). That interpreter would fgets(stdin, ...) which returned NULL instead of blocking because I was setting up my pipes incorrectly. It works great now, and my additions (a ncurses-based GUI) now has the interpreter running in a pane. Of course, this has become a bit of a moot point, because I realize I need access to the memory space of the interpreter, so I'll need to move to pthreads and the fork will be a moot issue. – Robert Dec 04 '10 at 22:31
0

Somebody answered this... I don't know what happened to it. Considering I'm a newbie here, maybe I deleted it somehow? If so, I apologize, but they had the answer.

The solution is to not use the WIF* macros unless waitpid()>0, since apparently otherwise the 0 is considered a normal exit. I inserted a check into my code and it now works - thanks everyone for the editing pointers.

Robert
  • 6,412
  • 3
  • 24
  • 26
  • That was me. I was unsatisfied with the answer, though, and temporarily deleted it to complete it in privacy. I hope you don't mind the extra pointers. – dennycrane Dec 03 '10 at 18:21