Re: [Xen-devel] [PATCH 13/12] libxl: events: Break out libxl__pipe_nonblock, _close

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Tue, 2014-01-21 at 15:11 +0000, Ian Jackson wrote:
> Break out the pipe creation and destruction from the poller code
> into two new functions libxl__pipe_nonblock and libxl__pipe_close.
> 
> No overall functional difference other than minor differences in exact
> log messages.
> 
> Also move libxl__self_pipe_wakeup and libxl__self_pipe_eatall into the
> new pipe utilities section in libxl_event.c; this is pure code motion.

You also switched pipe() -> libxl_pipe(), which is fine but not
mentioned here.

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

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

> ---
>  tools/libxl/libxl_event.c    |  104 
> ++++++++++++++++++++++++++----------------
>  tools/libxl/libxl_internal.h |    9 ++++
>  2 files changed, 73 insertions(+), 40 deletions(-)
> 
> diff --git a/tools/libxl/libxl_event.c b/tools/libxl/libxl_event.c
> index bdef7ac..35a8f81 100644
> --- a/tools/libxl/libxl_event.c
> +++ b/tools/libxl/libxl_event.c
> @@ -1271,26 +1271,81 @@ int libxl_event_check(libxl_ctx *ctx, libxl_event 
> **event_r,
>  }
>  
>  /*
> - * Manipulation of pollers
> + * Utilities for pipes (specifically, useful for self-pipes)
>   */
>  
> -int libxl__poller_init(libxl_ctx *ctx, libxl__poller *p)
> +void libxl__pipe_close(int fds[2])
> +{
> +    if (fds[0] >= 0) close(fds[0]);
> +    if (fds[1] >= 0) close(fds[1]);
> +    fds[0] = fds[1] = -1;
> +}
> +
> +int libxl__pipe_nonblock(libxl_ctx *ctx, int fds[2])
>  {
>      int r, rc;
> -    p->fd_polls = 0;
> -    p->fd_rindices = 0;
>  
> -    r = pipe(p->wakeup_pipe);
> +    r = libxl_pipe(ctx, fds);
>      if (r) {
> -        LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "cannot create poller pipe");
> +        fds[0] = fds[1] = -1;
>          rc = ERROR_FAIL;
>          goto out;
>      }
>  
> -    rc = libxl_fd_set_nonblock(ctx, p->wakeup_pipe[0], 1);
> +    rc = libxl_fd_set_nonblock(ctx, fds[0], 1);
>      if (rc) goto out;
>  
> -    rc = libxl_fd_set_nonblock(ctx, p->wakeup_pipe[1], 1);
> +    rc = libxl_fd_set_nonblock(ctx, fds[1], 1);
> +    if (rc) goto out;
> +
> +    return 0;
> +
> + out:
> +    libxl__pipe_close(fds);
> +    return rc;
> +}
> +
> +int libxl__self_pipe_wakeup(int fd)
> +{
> +    static const char buf[1] = "";
> +
> +    for (;;) {
> +        int r = write(fd, buf, 1);
> +        if (r==1) return 0;
> +        assert(r==-1);
> +        if (errno == EINTR) continue;
> +        if (errno == EWOULDBLOCK) return 0;
> +        assert(errno);
> +        return errno;
> +    }
> +}
> +
> +int libxl__self_pipe_eatall(int fd)
> +{
> +    char buf[256];
> +    for (;;) {
> +        int r = read(fd, buf, sizeof(buf));
> +        if (r == sizeof(buf)) continue;
> +        if (r >= 0) return 0;
> +        assert(r == -1);
> +        if (errno == EINTR) continue;
> +        if (errno == EWOULDBLOCK) return 0;
> +        assert(errno);
> +        return errno;
> +    }
> +}
> +
> +/*
> + * Manipulation of pollers
> + */
> +
> +int libxl__poller_init(libxl_ctx *ctx, libxl__poller *p)
> +{
> +    int rc;
> +    p->fd_polls = 0;
> +    p->fd_rindices = 0;
> +
> +    rc = libxl__pipe_nonblock(ctx, p->wakeup_pipe);
>      if (rc) goto out;
>  
>      return 0;
> @@ -1302,8 +1357,7 @@ int libxl__poller_init(libxl_ctx *ctx, libxl__poller *p)
>  
>  void libxl__poller_dispose(libxl__poller *p)
>  {
> -    if (p->wakeup_pipe[1] > 0) close(p->wakeup_pipe[1]);
> -    if (p->wakeup_pipe[0] > 0) close(p->wakeup_pipe[0]);
> +    libxl__pipe_close(p->wakeup_pipe);
>      free(p->fd_polls);
>      free(p->fd_rindices);
>  }
> @@ -1347,36 +1401,6 @@ void libxl__poller_wakeup(libxl__egc *egc, 
> libxl__poller *p)
>      if (e) LIBXL__EVENT_DISASTER(egc, "cannot poke watch pipe", e, 0);
>  }
>  
> -int libxl__self_pipe_wakeup(int fd)
> -{
> -    static const char buf[1] = "";
> -
> -    for (;;) {
> -        int r = write(fd, buf, 1);
> -        if (r==1) return 0;
> -        assert(r==-1);
> -        if (errno == EINTR) continue;
> -        if (errno == EWOULDBLOCK) return 0;
> -        assert(errno);
> -        return errno;
> -    }
> -}
> -
> -int libxl__self_pipe_eatall(int fd)
> -{
> -    char buf[256];
> -    for (;;) {
> -        int r = read(fd, buf, sizeof(buf));
> -        if (r == sizeof(buf)) continue;
> -        if (r >= 0) return 0;
> -        assert(r == -1);
> -        if (errno == EINTR) continue;
> -        if (errno == EWOULDBLOCK) return 0;
> -        assert(errno);
> -        return errno;
> -    }
> -}
> -
>  /*
>   * Main event loop iteration
>   */
> diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
> index 8429448..9d17586 100644
> --- a/tools/libxl/libxl_internal.h
> +++ b/tools/libxl/libxl_internal.h
> @@ -509,6 +509,15 @@ _hidden char *libxl__strndup(libxl__gc *gc_opt, const 
> char *c, size_t n) NN1;
>   * string. (similar to a gc'd dirname(3)). */
>  _hidden char *libxl__dirname(libxl__gc *gc_opt, const char *s) NN1;
>  
> +/* Make a pipe and set both ends nonblocking.  On error, nothing
> + * is left open and both fds[]==-1, and a message is logged.
> + * Useful for self-pipes. */
> +_hidden int libxl__pipe_nonblock(libxl_ctx *ctx, int fds[2]);
> +/* Closes the pipe fd(s).  Either or both of fds[] may be -1 meaning
> + * `not open'.  Ignores any errors.  Sets fds[] to -1. */
> +_hidden void libxl__pipe_close(int fds[2]);
> +
> +
>  /* Each of these logs errors and returns a libxl error code.
>   * They do not mind if path is already removed.
>   * For _file, path must not be a directory; for _directory it must be. */



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