[libdrm,v2] libdrm: Use readdir instead of readdir_r to avoid build warnings

Message ID 20180320174823.6281-1-emil.l.velikov@gmail.com
State New
Headers show
Series
  • [libdrm,v2] libdrm: Use readdir instead of readdir_r to avoid build warnings
Related show

Commit Message

Emil Velikov March 20, 2018, 5:48 p.m.
From: John Stultz <john.stultz@linaro.org>

Building libdrm under AOSP, we see the following build warning:
external/libdrm/xf86drm.c:2861:12: warning: 'readdir_r' is deprecated: readdir_r is deprecated; use readdir instead [-Wdeprecated-declarations]
    while (readdir_r(sysdir, pent, &ent) == 0 && ent != NULL) {
           ^

Building on Linux with glibc produces the same warning.
Thus, this patch replaces readdir_r with readdir.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102031
Cc: Robert Foss <robert.foss@collabora.com>
Cc: Rob Herring <robh@kernel.org>
Cc: Stefan Schake <stschake@gmail.com>
Cc: John Stultz <john.stultz@linaro.org>
Cc: Eric Engestrom <eric.engestrom@imgtec.com>
Signed-off-by: John Stultz <john.stultz@linaro.org>
Reviewed-by: Emil Velikov <emil.velikov@collabora.com>
[Emil Velikov: remove unused variables, Eric]
Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
---
 xf86drm.c | 22 +++-------------------
 1 file changed, 3 insertions(+), 19 deletions(-)

Comments

Eric Engestrom March 20, 2018, 6:17 p.m. | #1
On Tuesday, 2018-03-20 17:48:23 +0000, Emil Velikov wrote:
> From: John Stultz <john.stultz@linaro.org>
> 
> Building libdrm under AOSP, we see the following build warning:
> external/libdrm/xf86drm.c:2861:12: warning: 'readdir_r' is deprecated: readdir_r is deprecated; use readdir instead [-Wdeprecated-declarations]
>     while (readdir_r(sysdir, pent, &ent) == 0 && ent != NULL) {
>            ^
> 
> Building on Linux with glibc produces the same warning.
> Thus, this patch replaces readdir_r with readdir.
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102031
> Cc: Robert Foss <robert.foss@collabora.com>
> Cc: Rob Herring <robh@kernel.org>
> Cc: Stefan Schake <stschake@gmail.com>
> Cc: John Stultz <john.stultz@linaro.org>
> Cc: Eric Engestrom <eric.engestrom@imgtec.com>
> Signed-off-by: John Stultz <john.stultz@linaro.org>
> Reviewed-by: Emil Velikov <emil.velikov@collabora.com>
> [Emil Velikov: remove unused variables, Eric]
> Signed-off-by: Emil Velikov <emil.velikov@collabora.com>

I think that's pretty much exactly the patch I have at home :)
(and that I forgot to send out last night)

Reviewed-by: Eric Engestrom <eric.engestrom@imgtec.com>

> ---
>  xf86drm.c | 22 +++-------------------
>  1 file changed, 3 insertions(+), 19 deletions(-)
> 
> diff --git a/xf86drm.c b/xf86drm.c
> index 9328bf5a..97f87e02 100644
> --- a/xf86drm.c
> +++ b/xf86drm.c
> @@ -2822,12 +2822,11 @@ static char *drmGetMinorNameForFD(int fd, int type)
>  {
>  #ifdef __linux__
>      DIR *sysdir;
> -    struct dirent *pent, *ent;
> +    struct dirent *ent;
>      struct stat sbuf;
>      const char *name = drmGetMinorName(type);
>      int len;
>      char dev_name[64], buf[64];
> -    long name_max;
>      int maj, min;
>  
>      if (!name)
> @@ -2850,30 +2849,16 @@ static char *drmGetMinorNameForFD(int fd, int type)
>      if (!sysdir)
>          return NULL;
>  
> -    name_max = fpathconf(dirfd(sysdir), _PC_NAME_MAX);
> -    if (name_max == -1)
> -        goto out_close_dir;
> -
> -    pent = malloc(offsetof(struct dirent, d_name) + name_max + 1);
> -    if (pent == NULL)
> -         goto out_close_dir;
> -
> -    while (readdir_r(sysdir, pent, &ent) == 0 && ent != NULL) {
> +    while ((ent = readdir(sysdir))) {
>          if (strncmp(ent->d_name, name, len) == 0) {
>              snprintf(dev_name, sizeof(dev_name), DRM_DIR_NAME "/%s",
>                   ent->d_name);
>  
> -            free(pent);
>              closedir(sysdir);
> -
>              return strdup(dev_name);
>          }
>      }
> -
> -    free(pent);
> -
> -out_close_dir:
> -    closedir(sysdir);
> +    return NULL;

This return and the hunk below are the one bit that I didn't have in my
patch; it's unreachable on the other branch, so you're entirely right to
move it!

>  #else
>      struct stat sbuf;
>      char buf[PATH_MAX + 1];
> @@ -2914,7 +2899,6 @@ out_close_dir:
>  
>      return strdup(buf);
>  #endif
> -    return NULL;
>  }
>  
>  char *drmGetPrimaryDeviceNameFromFd(int fd)
> -- 
> 2.16.0
>
Emil Velikov March 22, 2018, 4:51 p.m. | #2
On 20 March 2018 at 18:17, Eric Engestrom <eric.engestrom@imgtec.com> wrote:
> On Tuesday, 2018-03-20 17:48:23 +0000, Emil Velikov wrote:
>> From: John Stultz <john.stultz@linaro.org>
>>
>> Building libdrm under AOSP, we see the following build warning:
>> external/libdrm/xf86drm.c:2861:12: warning: 'readdir_r' is deprecated: readdir_r is deprecated; use readdir instead [-Wdeprecated-declarations]
>>     while (readdir_r(sysdir, pent, &ent) == 0 && ent != NULL) {
>>            ^
>>
>> Building on Linux with glibc produces the same warning.
>> Thus, this patch replaces readdir_r with readdir.
>>
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102031
>> Cc: Robert Foss <robert.foss@collabora.com>
>> Cc: Rob Herring <robh@kernel.org>
>> Cc: Stefan Schake <stschake@gmail.com>
>> Cc: John Stultz <john.stultz@linaro.org>
>> Cc: Eric Engestrom <eric.engestrom@imgtec.com>
>> Signed-off-by: John Stultz <john.stultz@linaro.org>
>> Reviewed-by: Emil Velikov <emil.velikov@collabora.com>
>> [Emil Velikov: remove unused variables, Eric]
>> Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
>
> I think that's pretty much exactly the patch I have at home :)
> (and that I forgot to send out last night)
>
> Reviewed-by: Eric Engestrom <eric.engestrom@imgtec.com>
>
Thanks pushed to master.

-Emil
John Stultz March 26, 2018, 11:31 p.m. | #3
On Thu, Mar 22, 2018 at 9:51 AM, Emil Velikov <emil.l.velikov@gmail.com> wrote:
> On 20 March 2018 at 18:17, Eric Engestrom <eric.engestrom@imgtec.com> wrote:
>> On Tuesday, 2018-03-20 17:48:23 +0000, Emil Velikov wrote:
>>> From: John Stultz <john.stultz@linaro.org>
>>>
>>> Building libdrm under AOSP, we see the following build warning:
>>> external/libdrm/xf86drm.c:2861:12: warning: 'readdir_r' is deprecated: readdir_r is deprecated; use readdir instead [-Wdeprecated-declarations]
>>>     while (readdir_r(sysdir, pent, &ent) == 0 && ent != NULL) {
>>>            ^
>>>
>>> Building on Linux with glibc produces the same warning.
>>> Thus, this patch replaces readdir_r with readdir.
>>>
>>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102031
>>> Cc: Robert Foss <robert.foss@collabora.com>
>>> Cc: Rob Herring <robh@kernel.org>
>>> Cc: Stefan Schake <stschake@gmail.com>
>>> Cc: John Stultz <john.stultz@linaro.org>
>>> Cc: Eric Engestrom <eric.engestrom@imgtec.com>
>>> Signed-off-by: John Stultz <john.stultz@linaro.org>
>>> Reviewed-by: Emil Velikov <emil.velikov@collabora.com>
>>> [Emil Velikov: remove unused variables, Eric]
>>> Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
>>
>> I think that's pretty much exactly the patch I have at home :)
>> (and that I forgot to send out last night)
>>
>> Reviewed-by: Eric Engestrom <eric.engestrom@imgtec.com>
>>
> Thanks pushed to master.

Sorry again for the slow response here, but an after-the-fact thanks
for sending out the much improved version!

I really appreciate it!
-john

Patch

diff --git a/xf86drm.c b/xf86drm.c
index 9328bf5a..97f87e02 100644
--- a/xf86drm.c
+++ b/xf86drm.c
@@ -2822,12 +2822,11 @@  static char *drmGetMinorNameForFD(int fd, int type)
 {
 #ifdef __linux__
     DIR *sysdir;
-    struct dirent *pent, *ent;
+    struct dirent *ent;
     struct stat sbuf;
     const char *name = drmGetMinorName(type);
     int len;
     char dev_name[64], buf[64];
-    long name_max;
     int maj, min;
 
     if (!name)
@@ -2850,30 +2849,16 @@  static char *drmGetMinorNameForFD(int fd, int type)
     if (!sysdir)
         return NULL;
 
-    name_max = fpathconf(dirfd(sysdir), _PC_NAME_MAX);
-    if (name_max == -1)
-        goto out_close_dir;
-
-    pent = malloc(offsetof(struct dirent, d_name) + name_max + 1);
-    if (pent == NULL)
-         goto out_close_dir;
-
-    while (readdir_r(sysdir, pent, &ent) == 0 && ent != NULL) {
+    while ((ent = readdir(sysdir))) {
         if (strncmp(ent->d_name, name, len) == 0) {
             snprintf(dev_name, sizeof(dev_name), DRM_DIR_NAME "/%s",
                  ent->d_name);
 
-            free(pent);
             closedir(sysdir);
-
             return strdup(dev_name);
         }
     }
-
-    free(pent);
-
-out_close_dir:
-    closedir(sysdir);
+    return NULL;
 #else
     struct stat sbuf;
     char buf[PATH_MAX + 1];
@@ -2914,7 +2899,6 @@  out_close_dir:
 
     return strdup(buf);
 #endif
-    return NULL;
 }
 
 char *drmGetPrimaryDeviceNameFromFd(int fd)