Re: [Xen-devel] [PATCH 24/29] libvchan: handle libxc evtchn failures properly in init functions

[Matthew Daley <mattjd@xxxxxxxxx>]

On Wed, Oct 30, 2013 at 8:52 PM, Matthew Daley <mattjd@xxxxxxxxx> wrote:
> Also, remove now-unnecessary check from close function.
>
> Coverity-ID: 1055609
> Coverity-ID: 1055610
> Coverity-ID: 1055611
> Signed-off-by: Matthew Daley <mattjd@xxxxxxxxx>

I should clarify: the base reason for this patch is that
ctrl->event_port is a uint32_t (ie. unsigned), so the current checks
on it for negative error results, non-negative port presence etc. are
incorrect.

I can spin a v2 with this mentioned if desired.

- Matthew

> ---
>  tools/libvchan/init.c |   47 +++++++++++++++++++++++++++++++++++++++--------
>  tools/libvchan/io.c   |    2 +-
>  2 files changed, 40 insertions(+), 9 deletions(-)
>
> diff --git a/tools/libvchan/init.c b/tools/libvchan/init.c
> index 0c7cff6..314b1f6 100644
> --- a/tools/libvchan/init.c
> +++ b/tools/libvchan/init.c
> @@ -215,15 +215,30 @@ static int init_gnt_cli(struct libxenvchan *ctrl, int 
> domain, uint32_t ring_ref)
>
>  static int init_evt_srv(struct libxenvchan *ctrl, int domain, 
> xentoollog_logger *logger)
>  {
> +       int port;
> +
>         ctrl->event = xc_evtchn_open(logger, 0);
>         if (!ctrl->event)
>                 return -1;
> -       ctrl->event_port = xc_evtchn_bind_unbound_port(ctrl->event, domain);
> -       if (ctrl->event_port < 0)
> -               return -1;
> +
> +       port = xc_evtchn_bind_unbound_port(ctrl->event, domain);
> +       if (port < 0)
> +               goto fail;
> +       ctrl->event_port = port;
> +
>         if (xc_evtchn_unmask(ctrl->event, ctrl->event_port))
> -               return -1;
> +               goto fail;
> +
>         return 0;
> +
> +fail:
> +       if (port >= 0)
> +               xc_evtchn_unbind(ctrl->event, port);
> +
> +       xc_evtchn_close(ctrl->event);
> +       ctrl->event = NULL;
> +
> +       return -1;
>  }
>
>  static int init_xs_srv(struct libxenvchan *ctrl, int domain, const char* 
> xs_base, int ring_ref)
> @@ -330,15 +345,31 @@ out:
>
>  static int init_evt_cli(struct libxenvchan *ctrl, int domain, 
> xentoollog_logger *logger)
>  {
> +       int port;
> +
>         ctrl->event = xc_evtchn_open(logger, 0);
>         if (!ctrl->event)
>                 return -1;
> -       ctrl->event_port = xc_evtchn_bind_interdomain(ctrl->event,
> +
> +       port = xc_evtchn_bind_interdomain(ctrl->event,
>                 domain, ctrl->event_port);
> -       if (ctrl->event_port < 0)
> -               return -1;
> -       xc_evtchn_unmask(ctrl->event, ctrl->event_port);
> +       if (port < 0)
> +               goto fail;
> +       ctrl->event_port = port;
> +
> +       if (xc_evtchn_unmask(ctrl->event, ctrl->event_port))
> +               goto fail;
> +
>         return 0;
> +
> +fail:
> +       if (port >= 0)
> +               xc_evtchn_unbind(ctrl->event, port);
> +
> +       xc_evtchn_close(ctrl->event);
> +       ctrl->event = NULL;
> +
> +       return -1;
>  }
>
>
> diff --git a/tools/libvchan/io.c b/tools/libvchan/io.c
> index 3c8d236..2383364 100644
> --- a/tools/libvchan/io.c
> +++ b/tools/libvchan/io.c
> @@ -337,7 +337,7 @@ void libxenvchan_close(struct libxenvchan *ctrl)
>                 }
>         }
>         if (ctrl->event) {
> -               if (ctrl->event_port >= 0 && ctrl->ring)
> +               if (ctrl->ring)
>                         xc_evtchn_notify(ctrl->event, ctrl->event_port);
>                 xc_evtchn_close(ctrl->event);
>         }
> --
> 1.7.10.4
>

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