On Fri, 29 Dec 2017 13:31:51 -0500
nerdopolis <[email protected]> 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.

> +                     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.

> +
>       /* 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

Attachment: pgp2sTO18kyIc.pgp
Description: OpenPGP digital signature

_______________________________________________
wayland-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Reply via email to