Re: [Xen-devel] [PATCH 3 of 9 v2] libxl: add libxl__forkexec function to libxl_exec

[Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>]

Roger Pau Monne writes ("[Xen-devel] [PATCH 3 of 9 v2] libxl: add 
libxl__forkexe> +        while (waitpid(pid, &status, 0) < 0) {
> +            if (errno != EINTR) {
> +                LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "waitpid failed\n");
> +                rc = -1;
> +                break;
> +            }
> +        }

It is good to see the exit status actually being retrieved and
checked.  But:

> +        if (WIFEXITED(status)) {
> +            rc = WEXITSTATUS(status);
> +        } else {
> +            rc = -1;
> +        }

I don't think this is the right thing to do with it.

Full and proper checking of a subprocess exit status involves:
  * checking for WIFSIGNALED too and perhaps calling strsignal
  * printing a message somewhere

I have found that the best way to deal with this is to split this up
into two parts:
   - Functions which pass back the wait status without interpreting it,
     and leaves checking it up to the caller
   - A generic function for reporting the meaning of an exit status
     to the log

It's also OK to have convenience functions which bundle these two
together, but the actual checking of an exit status involves logging.

Conveniently, I have already written the second function for you:
libxl_report_child_exitstatus :-).

Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel