Re: [PATCH libdrm] libdrm: Allow dynamic drm majors on linux

[Eric Engestrom <eric.engestrom@intel.com>]

On Friday, 2018-08-31 13:54:19 +0200, Thomas Hellstrom wrote:
> To determine whether a device node is a drm device node or not, the code
> currently compares the node's major number to the static drm major device
> number.
> 
> This breaks the standalone vmwgfx driver on XWayland dri clients,
> https://cgit.freedesktop.org/mesa/vmwgfx
> and any future attempt to introduce dynamic device numbers for drm.
> 
> So instead of checking for the device major, instead check for the presence
> of the /sys/dev/char/<major>:<minor>/device/drm directory.

Just FYI, this means it now matches /dev/fb0 too.

I don't think this will be an issue, but just pointing it out so people notice.

> 
> Signed-off-by: Thomas Hellstrom <thellstrom@vmware.com>
> ---
>  xf86drm.c | 31 +++++++++++++++++++++++--------
>  1 file changed, 23 insertions(+), 8 deletions(-)
> 
> diff --git a/xf86drm.c b/xf86drm.c
> index 7807dce9..4cfc5d3e 100644
> --- a/xf86drm.c
> +++ b/xf86drm.c
> @@ -2761,6 +2761,21 @@ char *drmGetDeviceNameFromFd(int fd)
>      return strdup(name);
>  }
>  
> +static bool
> +drmNodeIsDRM(int maj, int min)
> +{
> +#ifdef __linux__
> +  char path[64];
> +  struct stat sbuf;
> +
> +  snprintf(path, sizeof(path), "/sys/dev/char/%d:%d/device/drm",
> +	   maj, min);

Nit: mixing tabs and space on this line.

With that fixed:
Reviewed-by: Eric Engestrom <eric.engestrom@intel.com>

> +  return stat(path, &sbuf) == 0;
> +#else
> +  return maj == DRM_MAJOR;
> +#endif
> +}
> +
>  int drmGetNodeTypeFromFd(int fd)
>  {
>      struct stat sbuf;
> @@ -2772,7 +2787,7 @@ int drmGetNodeTypeFromFd(int fd)
>      maj = major(sbuf.st_rdev);
>      min = minor(sbuf.st_rdev);
>  
> -    if (maj != DRM_MAJOR || !S_ISCHR(sbuf.st_mode)) {
> +    if (!drmNodeIsDRM(maj, min) || !S_ISCHR(sbuf.st_mode)) {
>          errno = EINVAL;
>          return -1;
>      }
> @@ -2837,7 +2852,7 @@ static char *drmGetMinorNameForFD(int fd, int type)
>      maj = major(sbuf.st_rdev);
>      min = minor(sbuf.st_rdev);
>  
> -    if (maj != DRM_MAJOR || !S_ISCHR(sbuf.st_mode))
> +    if (!drmNodeIsDRM(maj, min) || !S_ISCHR(sbuf.st_mode))
>          return NULL;
>  
>      snprintf(buf, sizeof(buf), "/sys/dev/char/%d:%d/device/drm", maj, min);
> @@ -2871,7 +2886,7 @@ static char *drmGetMinorNameForFD(int fd, int type)
>      maj = major(sbuf.st_rdev);
>      min = minor(sbuf.st_rdev);
>  
> -    if (maj != DRM_MAJOR || !S_ISCHR(sbuf.st_mode))
> +    if (!drmNodeIsDRM(maj, min) || !S_ISCHR(sbuf.st_mode))
>          return NULL;
>  
>      switch (type) {
> @@ -3731,7 +3746,7 @@ process_device(drmDevicePtr *device, const char *d_name,
>      maj = major(sbuf.st_rdev);
>      min = minor(sbuf.st_rdev);
>  
> -    if (maj != DRM_MAJOR || !S_ISCHR(sbuf.st_mode))
> +    if (!drmNodeIsDRM(maj, min) || !S_ISCHR(sbuf.st_mode))
>          return -1;
>  
>      subsystem_type = drmParseSubsystemType(maj, min);
> @@ -3845,7 +3860,7 @@ int drmGetDevice2(int fd, uint32_t flags, drmDevicePtr *device)
>      maj = major(sbuf.st_rdev);
>      min = minor(sbuf.st_rdev);
>  
> -    if (maj != DRM_MAJOR || !S_ISCHR(sbuf.st_mode))
> +    if (!drmNodeIsDRM(maj, min) || !S_ISCHR(sbuf.st_mode))
>          return -EINVAL;
>  
>      node_type = drmGetMinorType(min);
> @@ -3911,7 +3926,7 @@ int drmGetDevice2(int fd, uint32_t flags, drmDevicePtr *device)
>      maj = major(sbuf.st_rdev);
>      min = minor(sbuf.st_rdev);
>  
> -    if (maj != DRM_MAJOR || !S_ISCHR(sbuf.st_mode))
> +    if (!drmNodeIsDRM(maj, min) || !S_ISCHR(sbuf.st_mode))
>          return -EINVAL;
>  
>      subsystem_type = drmParseSubsystemType(maj, min);
> @@ -4071,7 +4086,7 @@ char *drmGetDeviceNameFromFd2(int fd)
>      maj = major(sbuf.st_rdev);
>      min = minor(sbuf.st_rdev);
>  
> -    if (maj != DRM_MAJOR || !S_ISCHR(sbuf.st_mode))
> +    if (!drmNodeIsDRM(maj, min) || !S_ISCHR(sbuf.st_mode))
>          return NULL;
>  
>      snprintf(path, sizeof(path), "/sys/dev/char/%d:%d", maj, min);
> @@ -4097,7 +4112,7 @@ char *drmGetDeviceNameFromFd2(int fd)
>      maj = major(sbuf.st_rdev);
>      min = minor(sbuf.st_rdev);
>  
> -    if (maj != DRM_MAJOR || !S_ISCHR(sbuf.st_mode))
> +    if (!drmNodeIsDRM(maj, min) || !S_ISCHR(sbuf.st_mode))
>          return NULL;
>  
>      node_type = drmGetMinorType(min);
> -- 
> 2.18.0.rc1
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel