[PATCH weston] tests: Remove the redundant NULL checks

[Seedo Eldho Paul]

On Sep 4, 2015 1:48 AM, "Bryce Harrington" <bryce at osg.samsung.com> wrote:
>
> On Fri, Sep 04, 2015 at 12:17:18AM +0530, Seedo Eldho Paul wrote:
> > xzalloc is guaranteed to return a non-NULL value.
> >
> > Signed-off-by: Seedo Eldho Paul <seedoeldhopaul at gmail.com>
> > ---
> >  tests/internal-screenshot-test.c | 13 -------------
> >  1 file changed, 13 deletions(-)
>
> Good point.  The error handling code will never actually get executed.
>
> > diff --git a/tests/internal-screenshot-test.c
b/tests/internal-screenshot-test.c
> > index e72a695..a74ae7a 100644
> > --- a/tests/internal-screenshot-test.c
> > +++ b/tests/internal-screenshot-test.c
> > @@ -93,11 +93,6 @@ load_surface_from_png(const char *fname)
> >
> >       /* Disguise the cairo surface in a weston test surface */
> >       reference = xzalloc(sizeof *reference);
> > -     if (reference == NULL) {
> > -             perror("xzalloc reference");
> > -             cairo_surface_destroy(reference_cairo_surface);
> > -             return NULL;
> > -     }
>
> This will skip the surface destruction.  I suppose who cares, we're
> exiting anyway, but the xzalloc could be moved up prior to the surface
> creation, and make it a non-issue.
>
> >       reference->width =
cairo_image_surface_get_width(reference_cairo_surface);
> >       reference->height =
cairo_image_surface_get_height(reference_cairo_surface);
> >       stride = cairo_image_surface_get_stride(reference_cairo_surface);
> > @@ -115,12 +110,6 @@ load_surface_from_png(const char *fname)
> >       /* Allocate new buffer for our weston reference, and copy the
data from
> >          the cairo surface so we can destroy it */
> >       reference->data = xzalloc(source_data_size);
> > -     if (reference->data == NULL) {
> > -             perror("xzalloc reference data");
> > -             cairo_surface_destroy(reference_cairo_surface);
> > -             free(reference);
> > -             return NULL;
> > -     }
>
> Can't do that here though, since we depend on the surface to calculate
> source_data_size.  Also freeing reference in this case is a good idea.
> Maybe this is a case where we should plain zalloc instead?
>
> >       memcpy(reference->data,
> >              cairo_image_surface_get_data(reference_cairo_surface),
> >              source_data_size);
> > @@ -144,8 +133,6 @@ create_screenshot_surface(struct client *client)
> >  {
> >       struct surface* screenshot;
> >       screenshot = xzalloc(sizeof *screenshot);
> > -     if (screenshot == NULL)
> > -             return NULL;
> >       screenshot->wl_buffer = create_shm_buffer(client,
> >                                                 client->output->width,
> >                                                 client->output->height,
>
> Same as above, maybe should use zalloc instead.
>
> Thanks for reviewing this.  Sorry if these comments seem nit-picky for a
> just a test case...  The reason is that I plan to move a lot of this
> code out of this test into the testing infrastructure, so having it
> handle errors cleanly in this test case will be worthwhile in the longer
> run.
>

Thanks for the review. I agree with your rationale. I'll respin the patch
later today.

SEEDO

> Bryce
>
> > --
> > 1.9.1
> >
> > _______________________________________________
> > wayland-devel mailing list
> > wayland-devel at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/wayland-devel
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/wayland-devel/attachments/20150903/a32361fd/attachment.html>