Re: [Xen-devel] [PATCH 1/7] libxl: fork: Break out checked_waitpid

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Thu, 2014-01-16 at 17:22 +0000, Ian Jackson wrote:
> This is a simple error-handling wrapper for waitpid.  We're going to
> want to call waitpid somewhere else and this avoids some of the
> duplication.
> 
> No functional change in this patch.  (Technically, we used to check
> chldmode_ours again in the EINTR case, and don't now, but that can't
> have changed because we continuously hold the libxl ctx lock.)

I was going to ask if that outer while condition is a bit pointless
then, but I see that outside the context we drop and reacquire the lock
so it makes sense.

> Signed-off-by: Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>
> Cc: Jim Fehlig <jfehlig@xxxxxxxx>
> Cc: Ian Campbell <Ian.Campbell@xxxxxxxxxx>

Acked-by: Ian Campbell <ian.campbell@xxxxxxxxxx>

> ---
>  tools/libxl/libxl_fork.c |   26 ++++++++++++++++++--------
>  1 file changed, 18 insertions(+), 8 deletions(-)
> 
> diff --git a/tools/libxl/libxl_fork.c b/tools/libxl/libxl_fork.c
> index 4ae9f94..2252370 100644
> --- a/tools/libxl/libxl_fork.c
> +++ b/tools/libxl/libxl_fork.c
> @@ -155,6 +155,22 @@ int libxl__carefd_fd(const libxl__carefd *cf)
>   * Actual child process handling
>   */
>  
> +/* Like waitpid(,,WNOHANG) but handles all errors except ECHILD. */
> +static pid_t checked_waitpid(libxl__egc *egc, pid_t want, int *status)
> +{
> +    for (;;) {
> +        pid_t got = waitpid(want, status, WNOHANG);
> +        if (got != -1)
> +            return got;
> +        if (errno == ECHILD)
> +            return got;
> +        if (errno == EINTR)
> +            continue;
> +        LIBXL__EVENT_DISASTER(egc, "waitpid() failed", errno, 0);
> +        return 0;
> +    }
> +}
> +
>  static void sigchld_selfpipe_handler(libxl__egc *egc, libxl__ev_fd *ev,
>                                       int fd, short events, short revents);
>  
> @@ -331,16 +347,10 @@ static void sigchld_selfpipe_handler(libxl__egc *egc, 
> libxl__ev_fd *ev,
>  
>      while (chldmode_ours(CTX, 0) /* in case the app changes the mode */) {
>          int status;
> -        pid_t pid = waitpid(-1, &status, WNOHANG);
> -
> -        if (pid == 0) return;
> +        pid_t pid = checked_waitpid(egc, -1, &status);
>  
> -        if (pid == -1) {
> -            if (errno == ECHILD) return;
> -            if (errno == EINTR) continue;
> -            LIBXL__EVENT_DISASTER(egc, "waitpid() failed", errno, 0);
> +        if (pid == 0 || pid == -1 /* ECHILD */)
>              return;
> -        }
>  
>          int rc = childproc_reaped(egc, pid, status);
>  



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel