[PATCH] xwayland: Always free reply from xcb_get_property_reply()

[Bryce Harrington]

On Wed, Jul 01, 2015 at 11:19:09AM +0200, Marek Chalupa wrote:
> On Wed, Jul 1, 2015 at 6:35 AM, Bryce Harrington <bryce at osg.samsung.com>
> wrote:
> 
> > Signed-off-by: Bryce Harrington <bryce at osg.samsung.com>
> > ---
> >  xwayland/selection.c | 10 +++++++---
> >  1 file changed, 7 insertions(+), 3 deletions(-)
> >
> > diff --git a/xwayland/selection.c b/xwayland/selection.c
> > index 572a0b5..451b915 100644
> > --- a/xwayland/selection.c
> > +++ b/xwayland/selection.c
> > @@ -119,8 +119,9 @@ weston_wm_get_incr_chunk(struct weston_wm *wm)
> >         } else {
> >                 weston_log("transfer complete\n");
> >                 close(wm->data_source_fd);
> > -               free(reply);
> >         }
> > +
> > +       free(reply);
> >  }
> >
> >  struct x11_data_source {
> > @@ -192,8 +193,10 @@ weston_wm_get_selection_targets(struct weston_wm *wm)
> >         }
> >
> >         source = malloc(sizeof *source);
> > -       if (source == NULL)
> > +       if (source == NULL) {
> > +               free(reply);
> >                 return;
> > +       }
> >
> >         wl_signal_init(&source->base.destroy_signal);
> >         source->base.accept = data_source_accept;
> > @@ -237,12 +240,13 @@ weston_wm_get_selection_data(struct weston_wm *wm)
> >         if (reply->type == wm->atom.incr) {
> >                 dump_property(wm, wm->atom.wl_selection, reply);
> >                 wm->incr = 1;
> > -               free(reply);
> >         } else {
> >                 dump_property(wm, wm->atom.wl_selection, reply);
> >                 wm->incr = 0;
> >                 weston_wm_write_property(wm, reply);
> >         }
> > +
> > +       free(reply);
> >  }
> >
> >  static void
> > --
> > 1.9.1
> >
> > _______________________________________________
> > wayland-devel mailing list
> > wayland-devel at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/wayland-devel
> >
> 
> Just wondering: does xcb_get_property_reply always return non-NULL? Because
> there's no check for that before dereferencing the reply.
> 
> Reviewed-by: Marek Chalupa <mchqwerty at gmail.com>

Thanks for the review.  You're right, those return values probably
should be checked.

The man page doesn't explicitly say NULL is returned, but it does
mention the need for error checking, and the example it provides
definitely does show a null pointer check.

I'll follow up with a separate patch to add those checks.

Thanks,
Bryce