[PATCH libdrm 1/3] xf86drm: adjust device node path for minor base

[Emil Velikov]

On 12 December 2016 at 15:07, Jonathan Gray <jsg at 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 at 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 at 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