[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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



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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.