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

[Emil Velikov]

On 23 February 2015 at 14:35, Frank Binns <frank.binns at imgtec.com> wrote:
> 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 at imgtec.com>
>> Cc: Daniel Vetter <daniel.vetter at ffwll.ch>
>> Cc: David Herrmann <dh.herrmann at googlemail.com>
>> Signed-off-by: Emil Velikov <emil.l.velikov at 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',
Most places that I've seen call "/dev/dri/cardX" master node, although
with the introduction of DRM_NODE_PRIMARY I believe your suggestion
may be better.

> 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.
>
I was thinking about this initially then decided to keep the
Render/MasterFD explicit. Mainly because I'm don't know how cumbersome
the *BSD implementation will be.

But with that said it none of the BSD guys objects within 1-2 weeks
I'll go with your suggestion.

Thanks
Emil