On Monday, January 22, 2018 4:50:35 AM EST Pekka Paalanen wrote: > On Fri, 29 Dec 2017 13:31:51 -0500 > nerdopolis <bluescreen_aven...@verizon.net> wrote: > > > This adds a function to detect the first framebuffer device in the > > current seat. Instead of hardcoding /dev/fb0, use udev to find the > > first framebuffer device in the seat. > > --- > > libweston/compositor-fbdev.c | 45 > > +++++++++++++++++++++++++++++++++++++++++--- > > 1 file changed, 42 insertions(+), 3 deletions(-) > > > > diff --git a/libweston/compositor-fbdev.c b/libweston/compositor-fbdev.c > > index 39668aa8..34e6c2f3 100644 > > --- a/libweston/compositor-fbdev.c > > +++ b/libweston/compositor-fbdev.c > > @@ -711,6 +711,42 @@ fbdev_restore(struct weston_compositor *compositor) > > weston_launcher_restore(compositor->launcher); > > } > > > > +static char * > > +find_framebuffer_device(struct fbdev_backend *b, const char *seat) > > +{ > > + struct udev_enumerate *e; > > + struct udev_list_entry *entry; > > + const char *path, *device_seat; > > + char *fb_device; > > + struct udev_device *device; > > + > > + e = udev_enumerate_new(b->udev); > > + udev_enumerate_add_match_subsystem(e, "graphics"); > > + udev_enumerate_add_match_sysname(e, "fb[0-9]*"); > > + > > + udev_enumerate_scan_devices(e); > > + fb_device = NULL; > > + udev_list_entry_foreach(entry, udev_enumerate_get_list_entry(e)) { > > + > > + path = udev_list_entry_get_name(entry); > > + device = udev_device_new_from_syspath(b->udev, path); > > + if (!device) > > + continue; > > + device_seat = udev_device_get_property_value(device, "ID_SEAT"); > > + if (!device_seat) > > + device_seat = default_seat; > > + if (!strcmp(device_seat, seat)) { > > + fb_device = udev_device_get_devnode(device); > > + udev_enumerate_unref(e); > > Hi, > > seems like this should be udev_device_unref() instead, otherwise > 'e' is unreffed twice and 'device' is leaked. > Patch 7 improves this function somewhat, but I don't really know how to squash 5 and 7 into 5 without squashing 6... ...I guess I could just copy and paste that manually...
With patch 7 applied on top of this, is the only issue that I just need to do fb_device = strdup(find_device); udev_device_unref(device); to be able to unref device ? > > + break; > > + } > > + udev_device_unref(device); > > + } > > + > > + udev_enumerate_unref(e); > > + return fb_device; > > +} > > + > > static struct fbdev_backend * > > fbdev_backend_create(struct weston_compositor *compositor, > > struct weston_fbdev_backend_config *param) > > @@ -743,6 +779,11 @@ fbdev_backend_create(struct weston_compositor > > *compositor, > > goto out_compositor; > > } > > > > + if (!param->device) > > + param->device=find_framebuffer_device(backend, seat_id); > > + if (!param->device) > > + param->device=strdup("/dev/fb0"); > > Missing spaces around '=' on both lines. > > The assigned string is leaked, there is no free() for it. However, one > must take care to not free() the string that came as an argument via > weston_fbdev_backend_config struct. > Not sure what I need to do here, regarding the free() TBH. The issue is with the param->device=strdup("/dev/fb0"); line correct? Is the issue that the hardcoded "/dev/fb0" string that is strdup'ed stuck in memory? > > + > > /* Set up the TTY. */ > > backend->session_listener.notify = session_notify; > > wl_signal_add(&compositor->session_signal, > > @@ -790,10 +831,8 @@ out_compositor: > > static void > > config_init_to_defaults(struct weston_fbdev_backend_config *config) > > { > > - /* TODO: Ideally, available frame buffers should be enumerated using > > - * udev, rather than passing a device node in as a parameter. */ > > config->tty = 0; /* default to current tty */ > > - config->device = "/dev/fb0"; /* default frame buffer */ > > + config->device = NULL; > > config->seat_id = NULL; > > } > > > > Otherwise looks good to me. > > > Thanks, > pq > _______________________________________________ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel