A simple fork with an ugly waitpid

Question

This is my first take on fork, exec and waitpid. I didn't quite understand why I had to use a while loop with waitpid. I discovered that waitpid sets errno to EINTR which is translated to this:

#define EINTR 4 /* Interrupted system call */

And I saw a suggestion in comp.unix.programmer archives to use a while loop for waitpid.

My implementation works on OSX 10.10.3 but it doesn't quite satisfy me. Can you provide suggestions?

main.c

#include <stdio.h>
#include <unistd.h>
#include <sys/wait.h>
#include <errno.h>
#include "common.h"
int main(int argc, const char * argv[]) {
 int exit_status_raw;
 pid_t pid = fork();
 if (pid < 0) {
 printerr("Forking failed");
 return -1;
 }
 if (pid == 0) {
 printf("CHILD says Hello\n");
 execlp("./child", "./child", NULL);
 printerr("exec failed");
 return -1;
 }
 if (pid > 0) {
 printf("PARENT says Hello\n");
 while ((pid = waitpid(pid, &exit_status_raw, 0)) == -1) {
 switch (errno) {
 case EINTR:
 printerr("waitpid failed with EINTR. Retrying...");
 continue;
 default:
 printerr("waitpid failed");
 break;
 }
 }
 printf("Child exited with %d\n", WEXITSTATUS(exit_status_raw));
 }
 return 0;
}

child.c

#include <stdio.h>
#include "common.h"
int main(int argc, const char * argv[]) {
 printf("This is a time consuming computation\n");
 // ...
 printerr("Sorry. I failed to compute");
 return 127;
}

common.h

#ifndef my_common_h
#define my_common_h
void printerr(const char *message) {
 fprintf(stderr, "%s\n", message);
}
#endif

• Cengiz Can: I've found an explanation on why certain blocking system calls are EINTRupttible in this blog post: EINTR and What It Is Good For

Answer 1

I didn't quite understand why I had to use a while loop with waitpid

To loop or not to loop should be a well-thought design decision.

The main reason for waiting in the loop is to monitor child's life for events other than exit (e.g. when the WUNTRACED option is set). Since you do not set this option, the only remaining reason is to deal with interrupts. It is really a minor one, and often you want to be interrupted (consider for example a child entering a non-alertable sleep).

The way you are dealing with errors other than EINTR is not correct. break only breaks the switch, and the code keeps executing the loop. Since the errors like EINVAL are non-recoverable, the program would get stuck in the loop forever (hitting the same error over and over again).

• Cengiz Can: In the end, I had to embrace the while loop. Here's a very informative blog post from the author of ZeroMQ about this 250bpm.com/blog:12

Answer 2

• You could consider terminating with the exit codes from <sysexits.h>, especially if the client is expecting them. For instance, if fork() fails, return or call exit() with EX_OSERR.

• It may be helpful to make the 127 error code a macro with a relevant name, since it's probably not a very commonly-known error code, especially returned from main(). I assume it's being used correctly (based on this post).

• It would look clearer to put execlp()'s return value in a conditional, rather than always printing an error afterwards. Since the exec functions only return a value (-1) on error, have the error message displayed only if this value gets returned. Of course, have errno printed out as well.

• You should also check to see if the child process has terminated from a signal. This can be done by calling WTERMSIG() and displaying its returned signal value. That can be determined by first checking the return value of WIFEXITED() (before doing the other checks).

  Example:

  int status;
  // do checks with waitpid()
  if (WIFEXITED(status))
  {
   fprintf(stderr, "the process exited with code %d\n", status); 
   exit(WEXITSTATUS(status));
  }
  else
  {
   fprintf(stderr, "the process died with signal %d\n", WTERMSIG(status));
   exit(EX_OSERR);
  }
  

• I don't see a need for printerr() when you can just use perror(), which is already provided in <stdio.h> and does more than your function. Regardless, it's a little redundant to put "failed" into the argument. Clients receiving such a message should already be able to see that it's some kind of error, and it's automatically printed to stderr.

• Instead of just having default provide an ambiguous error message, have it display the current value of errno with perror(). Again, your function doesn't do very much.

• Cengiz Can: I didn't know about the perror. And thank you for reminding sysexits.h. The case of failing waitpid is the actual thing that bothers me.