Hello Pekka,

The patch look good, but leave a leak in backend_init: the param.device
is never freed.

This fixed you get my Reviewed-by ;)

Best regards

Le 28/04/2016 16:08, Pekka Paalanen a écrit :
> From: Pekka Paalanen <pekka.paala...@collabora.co.uk>
> 
> Ensuring that the pointer to the device path stays valid gets harder and
> harder with migrating to the libweston-style config handling. Therefore,
> make a copy of the string, private to struct fbdev_output.
> 
> Now the pointer passed in to fbdev_output_create() could be freed right
> after the call returns.
> 
> Cc: Benoit Gschwind <gschw...@gnu-log.net>
> Signed-off-by: Pekka Paalanen <pekka.paala...@collabora.co.uk>
> ---
>  src/compositor-fbdev.c | 13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/src/compositor-fbdev.c b/src/compositor-fbdev.c
> index 19c5e3b..e2f978b 100644
> --- a/src/compositor-fbdev.c
> +++ b/src/compositor-fbdev.c
> @@ -84,7 +84,7 @@ struct fbdev_output {
>       struct wl_event_source *finish_frame_timer;
>  
>       /* Frame buffer details. */
> -     const char *device; /* ownership shared with fbdev_parameters */
> +     char *device;
>       struct fbdev_screeninfo fb_info;
>       void *fb; /* length is fb_info.buffer_length */
>  
> @@ -470,7 +470,7 @@ fbdev_output_create(struct fbdev_backend *backend,
>               return -1;
>  
>       output->backend = backend;
> -     output->device = device;
> +     output->device = strdup(device);
>  
>       /* Create the frame buffer. */
>       fb_fd = fbdev_frame_buffer_open(output, device, &output->fb_info);
> @@ -554,6 +554,7 @@ out_hw_surface:
>       weston_output_destroy(&output->base);
>       fbdev_frame_buffer_destroy(output);
>  out_free:
> +     free(output->device);
>       free(output);
>  
>       return -1;
> @@ -580,6 +581,7 @@ fbdev_output_destroy(struct weston_output *base)
>       /* Remove the output. */
>       weston_output_destroy(&output->base);
>  
> +     free(output->device);
>       free(output);
>  }
>  
> @@ -607,7 +609,7 @@ fbdev_output_reenable(struct fbdev_backend *backend,
>       struct fbdev_output *output = to_fbdev_output(base);
>       struct fbdev_screeninfo new_screen_info;
>       int fb_fd;
> -     const char *device;
> +     char *device;
>  
>       weston_log("Re-enabling fbdev output.\n");
>  
> @@ -634,9 +636,10 @@ fbdev_output_reenable(struct fbdev_backend *backend,
>               /* Remove and re-add the output so that resources depending on
>                * the frame buffer X/Y resolution (such as the shadow buffer)
>                * are re-initialised. */
> -             device = output->device;
> -             fbdev_output_destroy(base);
> +             device = strdup(output->device);
> +             fbdev_output_destroy(&output->base);
>               fbdev_output_create(backend, device);
> +             free(device);
>  
>               return 0;
>       }
> 
_______________________________________________
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Reply via email to