From: Lennart Jablonka <humm@ljabl.com>
Subject: [patch libdrm] re-fix drmGetMinorNameForFD
To: tech@openbsd.org, matthieu@openbsd.org
Date: Tue, 1 Oct 2024 20:20:49 +0000

OpenBSD added adding the base to the minor device number here:

	revision 1.44
	date: 2023/07/06 07:21:30;  author: matthieu;  state: Exp;  lines: +6 -2;  commitid: CZh4CsbG1zGTxqnw;
	fix drmGetMinorNameForFD(). tweaks and ok jsg@.
	For the gpu n, the main device node is /dev/dri/card<n> and the
	render device node is /dev/dri/renderD<n+drmGetMinorBase()> not
	/dev/dri/renderD<n>
	and miod@ checked that no port should be affected.

That's wrong and should be reverted.

For primary node /dev/dri/card<n>, the render node is indeed
called /dev/dri/renderD<n+base>.  But the render node's minor
number is <n+base> as well:

	$ ls -l /dev/dri
	total 0
	crw-------  1 humm  humm   87,   0 Sep 28 04:27 card0
	crw-------  1 root  wheel  87,   1 Sep 28 04:27 card1
	crw-------  1 root  wheel  87,   2 Sep 28 04:27 card2
	crw-------  1 root  wheel  87,   3 Sep 28 04:27 card3
	crw-------  1 humm  humm   87, 128 Sep 28 04:27 renderD128
	crw-------  1 root  wheel  87, 129 Sep 28 04:27 renderD129
	crw-------  1 root  wheel  87, 130 Sep 28 04:27 renderD130
	crw-------  1 root  wheel  87, 131 Sep 28 04:27 renderD131

The adding the base is implicit in the minor number; adding the
base to the render node's minor number yields a wrong path.
Here's an example:

	$ cat drm-name.c
	#include <fcntl.h>
	#include <stdio.h>
	#include <xf86drm.h>

	int
	main(void)
	{
	        int fd = open("/dev/dri/renderD128", O_RDONLY);
	        puts(drmGetRenderDeviceNameFromFd(fd));
	        puts(drmGetPrimaryDeviceNameFromFd(fd));
	        fd = open("/dev/dri/card0", O_RDONLY);
	        puts(drmGetPrimaryDeviceNameFromFd(fd));
	        puts(drmGetRenderDeviceNameFromFd(fd));
	}
	$ cc drm-name.c `pkg-config --cflags --libs libdrm`
	$ ./a.out
	/dev/dri/renderD256
	/dev/dri/card128
	/dev/dri/card0
	/dev/dri/renderD128

At least the first line in the output is wrong.  With this diff
the first line is correct:

	$ cc drm-name.c `pkg-config --cflags libdrm` /usr/xenocara/lib/libdrm/mk/libdrm/obj/libdrm.a
	ld: warning: xf86drm.c(xf86drm.o:(drmOpenMinor) in archive /usr/xenocara/lib/libdrm/mk/libdrm/obj/libdrm.a): warning: sprintf() is often misused, please use snprintf()
	$ ./a.out
	/dev/dri/renderD128
	/dev/dri/card128
	/dev/dri/card0
	/dev/dri/renderD0

Now, I don't know whether the second and fourth lines are wrong as
well--what the intended purpose of the function is.  On Linux, at
least, you're able to get the name of the corresponding
primary/render device for your render/primary file descriptor:

	linux$ cc drm-name.c `pkg-config --cflags --libs libdrm`
	linux$ ./a.out
	/dev/dri/renderD128
	/dev/dri/card0
	/dev/dri/card0
	/dev/dri/renderD128

Index: xf86drm.c
===================================================================
RCS file: /mnt/src/openbsd.cvs/xenocara/lib/libdrm/xf86drm.c,v
diff -u -p -r1.45 xf86drm.c
--- xf86drm.c	30 Jan 2024 06:16:37 -0000	1.45
+++ xf86drm.c	1 Oct 2024 17:25:38 -0000
@@ -3532,10 +3532,6 @@ static char *drmGetMinorNameForFD(int fd
      const char *dev_name = drmGetDeviceName(type);
      unsigned int maj, min;
      int n;
-    int base = drmGetMinorBase(type);
-
-    if (base < 0)
-        return NULL;
  
      if (fstat(fd, &sbuf))
          return NULL;
@@ -3549,7 +3545,7 @@ static char *drmGetMinorNameForFD(int fd
      if (!dev_name)
          return NULL;
  
-    n = snprintf(buf, sizeof(buf), dev_name, DRM_DIR_NAME, base + min);
+    n = snprintf(buf, sizeof(buf), dev_name, DRM_DIR_NAME, min);
      if (n == -1 || n >= sizeof(buf))
          return NULL;