On 12 December 2016 at 15:07, Jonathan Gray jsg@jsg.id.au wrote:

On Mon, Dec 12, 2016 at 02:10:31PM +0000, Emil Velikov wrote:

...

On 10 December 2016 at 05:52, Jonathan Gray jsg@jsg.id.au wrote:

...

When constructing a path to a device node the minor number retrieved from fstat needs to have the offset of the node type subtracted from it. Control and render node types have the same major as the primary node but each has their own block of minor types at fixed offsets.

Signed-off-by: Jonathan Gray jsg@jsg.id.au

xf86drm.c | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/xf86drm.c b/xf86drm.c index 2e8c956..6705605 100644 --- a/xf86drm.c +++ b/xf86drm.c @@ -2838,7 +2838,7 @@ out_close_dir: char buf[PATH_MAX + 1]; const char *dev_name; unsigned int maj, min;

• int n;

• int n, base;

  if (fstat(fd, &sbuf)) return NULL;

@@ -2863,7 +2863,11 @@ out_close_dir: return NULL; };

• n = snprintf(buf, sizeof(buf), dev_name, DRM_DIR_NAME, min);

• base = drmGetMinorBase(type);
• if (base < 0 || min < base)

min < base seems bogus, since it will never be true, right ? If so can we drop it please.

Same goes below and in 2/3. Emil

For drmGetDevice2() and drmGetDeviceNameFromFd2() yes as it should be caught by drmGetMinorType().

For drmGetMinorNameForFD() it would be possible for a minor number to be wrong and have it not caught as the type comes from an argument not inspection of the /dev node. A sanity check of the type argument could be made based on the minor found on disk but that ends up being the same kind of test.

K-kind of ...

There is no guarantee that that card0 relate to renderD128 and vice versa. Thus strictly speaking most/all the implementations that rely on drmGetMinorBase are buggy. IIIRC there was a discussion why those are not (and cannot) be made reliable, which inspired all the nasty sysfs parsing.

Nothing [new] should rely on drmOpen and friends, so we can ignore the drmGetMinorNameForFD corner cases. ... I hope

Thanks Emil

Implement a generic drmGetDeviceNameFromFd2() to use on non-linux systems without sysfs.

v2: remove min < base test as requested by Emil

Signed-off-by: Jonathan Gray jsg@jsg.id.au --- xf86drm.c | 44 ++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 42 insertions(+), 2 deletions(-)

diff --git a/xf86drm.c b/xf86drm.c index f6850aa2..f684c017 100644 --- a/xf86drm.c +++ b/xf86drm.c @@ -3627,7 +3627,47 @@ char *drmGetDeviceNameFromFd2(int fd) fclose(f); return device_name; #else -#warning "Missing implementation of drmGetDeviceNameFromFd2" - return NULL; + struct stat sbuf; + char node[PATH_MAX + 1]; + const char *dev_name; + int node_type; + int maj, min, n, base; + + 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; + + node_type = drmGetMinorType(min); + if (node_type == -1) + return NULL; + + switch (node_type) { + case DRM_NODE_PRIMARY: + dev_name = DRM_DEV_NAME; + break; + case DRM_NODE_CONTROL: + dev_name = DRM_CONTROL_DEV_NAME; + break; + case DRM_NODE_RENDER: + dev_name = DRM_RENDER_DEV_NAME; + break; + default: + return NULL; + }; + + base = drmGetMinorBase(node_type); + if (base < 0) + return NULL; + + n = snprintf(node, PATH_MAX, dev_name, DRM_DIR_NAME, min - base); + if (n == -1 || n >= PATH_MAX) + return NULL; + + return strdup(node); #endif }