Re: [PATCH libdrm v2] drm: add drmGet(Master|Render)NameFrom(Render|Master)FD functions

[Frank Binns <frank.binns@imgtec.com>]

Hi Emil,

On 23/02/15 12:22, Emil Velikov wrote:
> Currently most places assume reliable master <> render node mapping.
> Although this may work in some cases, it is not correct.
>
> Add a couple of helpers that hide the details and provide the name of
> the master/render device name, given an render/master FD.
>
> v2:
>  - Rename Device and Primary to Master (aka the /dev/dri/cardX device).
>  - Check for the file via readdir_r() rather than stat().
>  - Wrap the check into a single function.
>  - Return NULL for non-linux platforms.
>
> Cc: Frank Binns <frank.binns@imgtec.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: David Herrmann <dh.herrmann@googlemail.com>
> Signed-off-by: Emil Velikov <emil.l.velikov@gmail.com>
> ---
>  xf86drm.c | 82 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  xf86drm.h |  3 +++
>  2 files changed, 85 insertions(+)
>
> diff --git a/xf86drm.c b/xf86drm.c
> index e117bc6..d4a4dc6 100644
> --- a/xf86drm.c
> +++ b/xf86drm.c
> @@ -40,6 +40,8 @@
>  #include <string.h>
>  #include <strings.h>
>  #include <ctype.h>
> +#include <dirent.h>
> +#include <stddef.h>
>  #include <fcntl.h>
>  #include <errno.h>
>  #include <signal.h>
> @@ -522,6 +524,20 @@ static int drmGetMinorType(int minor)
>      }
>  }
>  
> +static const char *drmGetMinorName(int type)
> +{
> +    switch (type) {
> +    case DRM_NODE_PRIMARY:
> +        return "card";
> +    case DRM_NODE_CONTROL:
> +        return "controlD";
> +    case DRM_NODE_RENDER:
> +        return "renderD";
> +    default:
> +        return NULL;
> +    }
> +}
> +
>  /**
>   * Open the device by bus ID.
>   *
> @@ -2736,3 +2752,69 @@ int drmPrimeFDToHandle(int fd, int prime_fd, uint32_t *handle)
>  	return 0;
>  }
>  
> +static char *drmGetMinorNameForFD(int fd, int type)
> +{
> +#ifdef __linux__
> +	DIR *sysdir;
> +	struct dirent *pent, *ent;
> +	struct stat sbuf;
> +	const char *name = drmGetMinorName(type);
> +	const int len = strlen(name);
This will cause a segfault if 'name' is NULL.

> +	char dev_name[64], buf[64];
> +	long name_max;
> +	int maj, min;
> +
> +	if (!name)
> +		return NULL;
> +
> +	if (fstat(fd, &sbuf))
> +		return NULL;
> +
> +	maj = major(sbuf.st_rdev);
> +	min = minor(sbuf.st_rdev);
> +
> +	if (maj != DRM_MAJOR || !S_ISCHR(sbuf.st_mode))
> +		return NULL;
> +
> +	snprintf(buf, sizeof(buf), "/sys/dev/char/%d:%d/device/drm", maj, min);
> +
> +	sysdir = opendir(buf);
> +	if (!sysdir)
> +		return NULL;
> +
> +	name_max = fpathconf(dirfd(sysdir), _PC_NAME_MAX);
> +	if (name_max == -1)
> +		goto out_close_dir;
> +
> +	pent = malloc(offsetof(struct dirent, d_name) + name_max + 1);
> +	if (pent == NULL)
> +		 goto out_close_dir;
> +
> +	while (readdir_r(sysdir, pent, &ent) == 0 && ent != NULL) {
> +		if (strncmp(ent->d_name, name, len) == 0) {
> +			free(pent);
> +			closedir(sysdir);
> +
> +			snprintf(dev_name, sizeof(dev_name), DRM_DIR_NAME "/%s",
> +				 ent->d_name);
> +			return strdup(dev_name);
> +		}
> +	}
> +
> +	free(pent);
> +
> +out_close_dir:
> +	closedir(sysdir);
> +#endif
> +	return NULL;
> +}
> +
> +char *drmGetMasterNameFromRenderFD(int fd)
I think drmGetPrimaryDeviceNameFromFd would be more appropriate given
the node type is 'primary', the type of the fd doesn't matter afaics and
for consistency with other drmGet* functions. However, given that's a
bit of a mouthful I guess the 'Device' part could be dropped.

> +{
> +	return drmGetMinorNameForFD(fd, DRM_NODE_PRIMARY);
> +}
> +
> +char *drmGetRenderNameFromMasterFD(int fd)
As above, maybe drmGetRenderDeviceNameFromFd?

With those things changed/fixed you can have a:
Reviewed-by: Frank Binns <frank.binns@imgtec.com>

Thanks
Frank

> +{
> +	return drmGetMinorNameForFD(fd, DRM_NODE_RENDER);
> +}
> diff --git a/xf86drm.h b/xf86drm.h
> index afd38a1..5fdf27b 100644
> --- a/xf86drm.h
> +++ b/xf86drm.h
> @@ -749,6 +749,9 @@ extern int drmGetNodeTypeFromFd(int fd);
>  extern int drmPrimeHandleToFD(int fd, uint32_t handle, uint32_t flags, int *prime_fd);
>  extern int drmPrimeFDToHandle(int fd, int prime_fd, uint32_t *handle);
>  
> +extern char *drmGetRenderNameFromMasterFD(int fd);
> +extern char *drmGetMasterNameFromRenderFD(int fd);
> +
>  #if defined(__cplusplus) || defined(c_plusplus)
>  }
>  #endif

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel