[PATCH libdrm v2 04/10] xf86drm: Allocate drmDevicePtr's on stack
Robert Foss
robert.foss at collabora.com
Fri Jun 29 15:49:47 UTC 2018
LGTM
On 2018-06-29 17:20, Emil Velikov wrote:
> From: Emil Velikov <emil.velikov at collabora.com>
>
> Currently we dynamically allocate 16 pointers and reallocate more as
> needed.
>
> Instead, allocate the maximum number (256) on stack - the number is
> small enough and is unlikely to change in the foreseeable future.
>
> This allows us to simplify the error handling and even shed a few bytes
> off the final binary.
>
> v2:
> - add a define & description behind the magic 256 (Rob)
> - report error to strerr and skip when over 256 device nodes (Rob)
>
> Cc: Robert Foss <robert.foss at collabora.com>
> Signed-off-by: Emil Velikov <emil.velikov at collabora.com>
> Tested-by: Robert Foss <robert.foss at collabora.com> (v1)
> Reviewed-by: Robert Foss <robert.foss at collabora.com> (v1)
> Reviewed-by: Eric Engestrom <eric at engestrom.ch> (v1)
> ---
> xf86drm.c | 79 ++++++++++++++++---------------------------------------
> 1 file changed, 23 insertions(+), 56 deletions(-)
>
> diff --git a/xf86drm.c b/xf86drm.c
> index 114cf855..02da3e1f 100644
> --- a/xf86drm.c
> +++ b/xf86drm.c
> @@ -3766,6 +3766,13 @@ drm_device_has_rdev(drmDevicePtr device, dev_t find_rdev)
> return false;
> }
>
> +/*
> + * The kernel drm core has a number of places that assume maximum of
> + * 3x64 devices nodes. That's 64 for each of primary, control and
> + * render nodes. Rounded it up to 256 for simplicity.
> + */
> +#define MAX_DRM_NODES 256
> +
> /**
> * Get information about the opened drm device
> *
> @@ -3846,7 +3853,7 @@ int drmGetDevice2(int fd, uint32_t flags, drmDevicePtr *device)
>
> return 0;
> #else
> - drmDevicePtr *local_devices;
> + drmDevicePtr local_devices[MAX_DRM_NODES];
> drmDevicePtr d;
> DIR *sysdir;
> struct dirent *dent;
> @@ -3854,7 +3861,6 @@ int drmGetDevice2(int fd, uint32_t flags, drmDevicePtr *device)
> int subsystem_type;
> int maj, min;
> int ret, i, node_count;
> - int max_count = 16;
> dev_t find_rdev;
>
> if (drm_device_validate_flags(flags))
> @@ -3877,15 +3883,9 @@ int drmGetDevice2(int fd, uint32_t flags, drmDevicePtr *device)
> if (subsystem_type < 0)
> return subsystem_type;
>
> - local_devices = calloc(max_count, sizeof(drmDevicePtr));
> - if (local_devices == NULL)
> - return -ENOMEM;
> -
> sysdir = opendir(DRM_DIR_NAME);
> - if (!sysdir) {
> - ret = -errno;
> - goto free_locals;
> - }
> + if (!sysdir)
> + return -errno;
>
> i = 0;
> while ((dent = readdir(sysdir))) {
> @@ -3893,16 +3893,12 @@ int drmGetDevice2(int fd, uint32_t flags, drmDevicePtr *device)
> if (ret)
> continue;
>
> - if (i >= max_count) {
> - drmDevicePtr *temp;
> -
> - max_count += 16;
> - temp = realloc(local_devices, max_count * sizeof(drmDevicePtr));
> - if (!temp)
> - goto free_devices;
> - local_devices = temp;
> + if (i >= MAX_DRM_NODES) {
> + fprintf(stderr, "More than %d drm nodes detected. "
> + "Please report a bug - that should not happen.\n"
> + "Skipping extra nodes\n", MAX_DRM_NODES);
> + break;
> }
> -
> local_devices[i] = d;
> i++;
> }
> @@ -3921,18 +3917,9 @@ int drmGetDevice2(int fd, uint32_t flags, drmDevicePtr *device)
> }
>
> closedir(sysdir);
> - free(local_devices);
> if (*device == NULL)
> return -ENODEV;
> return 0;
> -
> -free_devices:
> - drmFreeDevices(local_devices, i);
> - closedir(sysdir);
> -
> -free_locals:
> - free(local_devices);
> - return ret;
> #endif
> }
>
> @@ -3968,25 +3955,18 @@ int drmGetDevice(int fd, drmDevicePtr *device)
> */
> int drmGetDevices2(uint32_t flags, drmDevicePtr devices[], int max_devices)
> {
> - drmDevicePtr *local_devices;
> + drmDevicePtr local_devices[MAX_DRM_NODES];
> drmDevicePtr device;
> DIR *sysdir;
> struct dirent *dent;
> int ret, i, node_count, device_count;
> - int max_count = 16;
>
> if (drm_device_validate_flags(flags))
> return -EINVAL;
>
> - local_devices = calloc(max_count, sizeof(drmDevicePtr));
> - if (local_devices == NULL)
> - return -ENOMEM;
> -
> sysdir = opendir(DRM_DIR_NAME);
> - if (!sysdir) {
> - ret = -errno;
> - goto free_locals;
> - }
> + if (!sysdir)
> + return -errno;
>
> i = 0;
> while ((dent = readdir(sysdir))) {
> @@ -3994,16 +3974,12 @@ int drmGetDevices2(uint32_t flags, drmDevicePtr devices[], int max_devices)
> if (ret)
> continue;
>
> - if (i >= max_count) {
> - drmDevicePtr *temp;
> -
> - max_count += 16;
> - temp = realloc(local_devices, max_count * sizeof(drmDevicePtr));
> - if (!temp)
> - goto free_devices;
> - local_devices = temp;
> + if (i >= MAX_DRM_NODES) {
> + fprintf(stderr, "More than %d drm nodes detected. "
> + "Please report a bug - that should not happen.\n"
> + "Skipping extra nodes\n", MAX_DRM_NODES);
> + break;
> }
> -
> local_devices[i] = device;
> i++;
> }
> @@ -4025,16 +4001,7 @@ int drmGetDevices2(uint32_t flags, drmDevicePtr devices[], int max_devices)
> }
>
> closedir(sysdir);
> - free(local_devices);
> return device_count;
> -
> -free_devices:
> - drmFreeDevices(local_devices, i);
> - closedir(sysdir);
> -
> -free_locals:
> - free(local_devices);
> - return ret;
> }
>
> /**
>
More information about the dri-devel
mailing list