Re: [PATCH libdrm 2/2] xf85drm: de-duplicate drmParse{Platform.Host1x}{Bus, Device}Info

On Wednesday, 2019-01-23 10:45:18 +0000, Emil Velikov wrote:

From: Emil Velikov emil.velikov@collabora.com

The functions are virtually identical, fold them up.

Signed-off-by: Emil Velikov emil.velikov@collabora.com

Assuming patch 1 is OK, and `foo` gets renamed to something better: Reviewed-by: Eric Engestrom eric.engestrom@intel.com

I don't know enough to review patch 1 though.

---

xf86drm.c | 98 +++++++++---------------------------------------------- 1 file changed, 15 insertions(+), 83 deletions(-)

diff --git a/xf86drm.c b/xf86drm.c index 374734eb..18c9693a 100644 --- a/xf86drm.c +++ b/xf86drm.c @@ -3508,7 +3508,7 @@ free_device: return ret; }

-static int drmParsePlatformBusInfo(int maj, int min, drmPlatformBusInfoPtr info) +static int drmParseOFBusInfo(int maj, int min, char *fullname) { #ifdef __linux__ char path[PATH_MAX + 1], *name, *foo; @@ -3532,19 +3532,18 @@ static int drmParsePlatformBusInfo(int maj, int min, drmPlatformBusInfoPtr info) foo++; }

• strncpy(info->fullname, foo, DRM_PLATFORM_DEVICE_NAME_LEN);
• info->fullname[DRM_PLATFORM_DEVICE_NAME_LEN - 1] = '\0';

• strncpy(fullname, foo, DRM_PLATFORM_DEVICE_NAME_LEN);

• fullname[DRM_PLATFORM_DEVICE_NAME_LEN - 1] = '\0'; free(name);

  return 0;

#else -#warning "Missing implementation of drmParsePlatformBusInfo" +#warning "Missing implementation of drmParseOFBusInfo" return -EINVAL; #endif }

-static int drmParsePlatformDeviceInfo(int maj, int min,

•                                  drmPlatformDeviceInfoPtr info)
  

+static int drmParseOFDeviceInfo(int maj, int min, char ***compatible) { #ifdef __linux__ char path[PATH_MAX + 1], *value, *foo; @@ -3562,8 +3561,8 @@ static int drmParsePlatformDeviceInfo(int maj, int min, count = 1; }

• info->compatible = calloc(count + 1, sizeof(*info->compatible));
• if (!info->compatible)

• *compatible = calloc(count + 1, sizeof(char *));

• if (!*compatible) return -ENOMEM;

  for (i = 0; i < count; i++) {

@@ -3587,19 +3586,19 @@ static int drmParsePlatformDeviceInfo(int maj, int min, free(value); }

•    info->compatible[i] = foo;
  

•    *compatible[i] = foo;
  

  }

  return 0;

free: while (i--)

•    free(info->compatible[i]);
  

•    free(*compatible[i]);
  
  

• free(info->compatible);

• free(*compatible); return err;

#else -#warning "Missing implementation of drmParsePlatformDeviceInfo" +#warning "Missing implementation of drmParseOFDeviceInfo" return -EINVAL; #endif } @@ -3622,7 +3621,7 @@ static int drmProcessPlatformDevice(drmDevicePtr *device,

 dev->businfo.platform = (drmPlatformBusInfoPtr)ptr;

• ret = drmParsePlatformBusInfo(maj, min, dev->businfo.platform);

• ret = drmParseOFBusInfo(maj, min, dev->businfo.platform->fullname); if (ret < 0) goto free_device;

@@ -3630,7 +3629,7 @@ static int drmProcessPlatformDevice(drmDevicePtr *device, ptr += sizeof(drmPlatformBusInfo); dev->deviceinfo.platform = (drmPlatformDeviceInfoPtr)ptr;

•    ret = drmParsePlatformDeviceInfo(maj, min, dev->deviceinfo.platform);
  

•    ret = drmParseOFDeviceInfo(maj, min, &dev->deviceinfo.platform->compatible);
     if (ret < 0)
         goto free_device;
  

  }

@@ -3644,73 +3643,6 @@ free_device: return ret; }

-static int drmParseHost1xBusInfo(int maj, int min, drmHost1xBusInfoPtr info) -{ -#ifdef __linux__

• char path[PATH_MAX + 1], *name;
• snprintf(path, sizeof(path), "/sys/dev/char/%d:%d/device", maj, min);
• name = sysfs_uevent_get(path, "OF_FULLNAME");
• if (!name)

•    return -ENOENT;
  

• strncpy(info->fullname, name, DRM_HOST1X_DEVICE_NAME_LEN);
• info->fullname[DRM_HOST1X_DEVICE_NAME_LEN - 1] = '\0';
• free(name);
• return 0;

-#else -#warning "Missing implementation of drmParseHost1xBusInfo"

• return -EINVAL;

-#endif -}

-static int drmParseHost1xDeviceInfo(int maj, int min,

•                                drmHost1xDeviceInfoPtr info)
  

-{ -#ifdef __linux__

• char path[PATH_MAX + 1], *value;
• unsigned int count, i;
• int err;
• snprintf(path, sizeof(path), "/sys/dev/char/%d:%d/device", maj, min);
• value = sysfs_uevent_get(path, "OF_COMPATIBLE_N");
• if (!value)

•    return -ENOENT;
  

• sscanf(value, "%u", &count);
• free(value);
• info->compatible = calloc(count + 1, sizeof(*info->compatible));
• if (!info->compatible)

•    return -ENOMEM;
  

• for (i = 0; i < count; i++) {

•    value = sysfs_uevent_get(path, "OF_COMPATIBLE_%u", i);
  

•    if (!value) {
  

•        err = -ENOENT;
  

•        goto free;
  

•    }
  

•    info->compatible[i] = value;
  

• }
• return 0;

-free:

• while (i--)

•    free(info->compatible[i]);
  

• free(info->compatible);
• return err;

-#else -#warning "Missing implementation of drmParseHost1xDeviceInfo"

• return -EINVAL;

-#endif -}

static int drmProcessHost1xDevice(drmDevicePtr *device, const char *node, int node_type, int maj, int min, bool fetch_deviceinfo, @@ -3729,7 +3661,7 @@ static int drmProcessHost1xDevice(drmDevicePtr *device,

 dev->businfo.host1x = (drmHost1xBusInfoPtr)ptr;

• ret = drmParseHost1xBusInfo(maj, min, dev->businfo.host1x);

• ret = drmParseOFBusInfo(maj, min, dev->businfo.host1x->fullname); if (ret < 0) goto free_device;

@@ -3737,7 +3669,7 @@ static int drmProcessHost1xDevice(drmDevicePtr *device, ptr += sizeof(drmHost1xBusInfo); dev->deviceinfo.host1x = (drmHost1xDeviceInfoPtr)ptr;

•    ret = drmParseHost1xDeviceInfo(maj, min, dev->deviceinfo.host1x);
  

•    ret = drmParseOFDeviceInfo(maj, min, &dev->deviceinfo.host1x->compatible);
     if (ret < 0)
         goto free_device;
  

  }

-- 2.20.1

---

dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel