On Thu, 28 Apr 2016 16:23:03 +0200 Quentin Glidic <[email protected]> wrote:
> On 28/04/2016 16:08, Pekka Paalanen wrote: > > From: Pekka Paalanen <[email protected]> > > > > 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 <[email protected]> > > Signed-off-by: Pekka Paalanen <[email protected]> > > --- > > 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); > > Maybe: > device = output->device; > output->device = NULL; > To avoid an strdup()? Hi Quentin, I did think about that, but if someone wanted to e.g. print the device string in the destroy function, that would be awkward. > > return 0; > > } > > > > > Another solution is to have fbdev_output_create() take ownership, so no > free() in _reenable(), and move the strdup() to backend_create(). Oh yes, that's true. I'm too lazy to bother if no-one really wants it to be changed, though. :-) > Whichever solution you pick (this patch, or one of the variants I > suggested): > > Reviewed-by: Quentin Glidic <[email protected]> > Thanks! I'll still wait Benoit's ack before landing these. Cheers, pq
pgpSy962yVdGA.pgp
Description: OpenPGP digital signature
_______________________________________________ wayland-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/wayland-devel
