Hey Emil,
On 2018-06-25 19:36, Emil Velikov wrote:
From: Emil Velikov emil.velikov@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.
Signed-off-by: Emil Velikov emil.velikov@collabora.com
xf86drm.c | 64 ++++++------------------------------------------------- 1 file changed, 6 insertions(+), 58 deletions(-)
diff --git a/xf86drm.c b/xf86drm.c index 114cf855..d4810740 100644 --- a/xf86drm.c +++ b/xf86drm.c @@ -3846,7 +3846,7 @@ int drmGetDevice2(int fd, uint32_t flags, drmDevicePtr *device)
return 0;
#else
- drmDevicePtr *local_devices;
- drmDevicePtr local_devices[256];
This number is seen later on in this patch, maybe it should be broken out into a define, since it's reused later on too at [1].
drmDevicePtr d; DIR *sysdir; struct dirent *dent;
@@ -3854,7 +3854,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 +3876,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 +3886,6 @@ int drmGetDevice2(int fd, uint32_t flags, drmDevicePtr *device) if (ret) continue;
if (i >= max_count) {
Is this check really not needded what exactly is it that defines 256 as the maximum?
From what I understand readdir(sysdir) is the call that defines how many devices will be looked through, and as far as I understand it can return an arbitrary number of files.
drmDevicePtr *temp;
max_count += 16;
temp = realloc(local_devices, max_count * sizeof(drmDevicePtr));
if (!temp)
goto free_devices;
local_devices = temp;
}
local_devices[i] = d; i++; }
@@ -3921,18 +3904,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 +3942,18 @@ int drmGetDevice(int fd, drmDevicePtr *device) */ int drmGetDevices2(uint32_t flags, drmDevicePtr devices[], int max_devices) {
- drmDevicePtr *local_devices;
- drmDevicePtr local_devices[256];'
[1]
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 +3961,6 @@ int drmGetDevices2(uint32_t flags, drmDevicePtr devices[], int max_devices) if (ret) continue;
if (i >= max_count) {
This out of bounds check should be dealt with too, if the above bounds check should be.
drmDevicePtr *temp;
max_count += 16;
temp = realloc(local_devices, max_count * sizeof(drmDevicePtr));
if (!temp)
goto free_devices;
local_devices = temp;
}
local_devices[i] = device; i++; }
@@ -4025,16 +3982,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; }
/**