[libdrm] Export drmCompareDevices

Commit Message

Adam Jackson May 4, 2017, 4:39 p.m. UTC

drmCompareBusInfo was almost this already, but it wasn't exported, its
name didn't match its functionality, and while it almost looks like it
was usable for sorting due to memcmp it wouldn't work if you had
multiple bus types. I don't really want to think about defining a
sensible sort order for bus types, so let's at least make it less of a
trap for the caller.

Invert its boolean sense to be 'true if equal', rename it to describe
the type it actually operates on, and export.

Signed-off-by: Adam Jackson <[e-mail]>
---
 xf86drm.c | 18 +++++++++---------
 xf86drm.h |  2 ++
 2 files changed, 11 insertions(+), 9 deletions(-)

Comments

Eric Anholt May 4, 2017, 5:15 p.m. UTC | #1

Adam Jackson <[e-mail]> writes:

> drmCompareBusInfo was almost this already, but it wasn't exported, its
> name didn't match its functionality, and while it almost looks like it
> was usable for sorting due to memcmp it wouldn't work if you had
> multiple bus types. I don't really want to think about defining a
> sensible sort order for bus types, so let's at least make it less of a
> trap for the caller.
>
> Invert its boolean sense to be 'true if equal', rename it to describe
> the type it actually operates on, and export.

I love flipping the comparison, but my only request would be to name it
drmDevicesEqual() instead, as Compare sounds a bit like memcmp() to me.
With that changed,

Reviewed-by: Eric Anholt <[e-mail]>

Emil Velikov May 4, 2017, 5:44 p.m. UTC | #2

On 4 May 2017 at 17:39, Adam Jackson <[e-mail]> wrote:
> drmCompareBusInfo was almost this already, but it wasn't exported, its
> name didn't match its functionality, and while it almost looks like it
> was usable for sorting due to memcmp it wouldn't work if you had
> multiple bus types. I don't really want to think about defining a
> sensible sort order for bus types, so let's at least make it less of a
> trap for the caller.
>
> Invert its boolean sense to be 'true if equal', rename it to describe
> the type it actually operates on, and export.
>
> Signed-off-by: Adam Jackson <[e-mail]>
Makes sense.

Reviewed-by: Emil Velikov <[e-mail]>

-Emil

Patch

diff --git a/xf86drm.c b/xf86drm.c
index 685cf69d..e584bdff 100644
--- a/xf86drm.c
+++ b/xf86drm.c
@@ -3029,32 +3029,32 @@  static int drmParsePciBusInfo(int maj, int min, drmPciBusInfoPtr info)
 #endif
 }
 
-static int drmCompareBusInfo(drmDevicePtr a, drmDevicePtr b)
+int drmCompareDevices(drmDevicePtr a, drmDevicePtr b)
 {
     if (a == NULL || b == NULL)
-        return -1;
+        return 0;
 
     if (a->bustype != b->bustype)
-        return -1;
+        return 0;
 
     switch (a->bustype) {
     case DRM_BUS_PCI:
-        return memcmp(a->businfo.pci, b->businfo.pci, sizeof(drmPciBusInfo));
+        return memcmp(a->businfo.pci, b->businfo.pci, sizeof(drmPciBusInfo)) == 0;
 
     case DRM_BUS_USB:
-        return memcmp(a->businfo.usb, b->businfo.usb, sizeof(drmUsbBusInfo));
+        return memcmp(a->businfo.usb, b->businfo.usb, sizeof(drmUsbBusInfo)) == 0;
 
     case DRM_BUS_PLATFORM:
-        return memcmp(a->businfo.platform, b->businfo.platform, sizeof(drmPlatformBusInfo));
+        return memcmp(a->businfo.platform, b->businfo.platform, sizeof(drmPlatformBusInfo)) == 0;
 
     case DRM_BUS_HOST1X:
-        return memcmp(a->businfo.host1x, b->businfo.host1x, sizeof(drmHost1xBusInfo));
+        return memcmp(a->businfo.host1x, b->businfo.host1x, sizeof(drmHost1xBusInfo)) == 0;
 
     default:
         break;
     }
 
-    return -1;
+    return 0;
 }
 
 static int drmGetNodeType(const char *name)
@@ -3669,7 +3669,7 @@  static void drmFoldDuplicatedDevices(drmDevicePtr local_devices[], int count)
 
     for (i = 0; i < count; i++) {
         for (j = i + 1; j < count; j++) {
-            if (drmCompareBusInfo(local_devices[i], local_devices[j]) == 0) {
+            if (drmCompareDevices(local_devices[i], local_devices[j])) {
                 local_devices[i]->available_nodes |= local_devices[j]->available_nodes;
                 node_type = log2(local_devices[j]->available_nodes);
                 memcpy(local_devices[i]->nodes[node_type],
diff --git a/xf86drm.h b/xf86drm.h
index d75ca8ce..14d2db73 100644
--- a/xf86drm.h
+++ b/xf86drm.h
@@ -851,6 +851,8 @@  extern void drmFreeDevices(drmDevicePtr devices[], int count);
 extern int drmGetDevice2(int fd, uint32_t flags, drmDevicePtr *device);
 extern int drmGetDevices2(uint32_t flags, drmDevicePtr devices[], int max_devices);
 
+extern int drmCompareDevices(drmDevicePtr a, drmDevicePtr b);
+
 #if defined(__cplusplus)
 }
 #endif