RFR: 8179887 - Build failure with glibc >= 2.24: error: 'int readdir_r(DIR*, dirent*, dirent**)' is deprecated

[B. Blaser]

Hi Alan,

On 28 June 2018 at 13:49, Alan Bateman <Alan.Bateman at oracle.com> wrote:
> On 20/06/2018 11:08, Pengfei Li wrote:
>>
>> Hi
>>
>> I have tried the patch (
>> http://mail.openjdk.java.net/pipermail/core-libs-dev/2018-May/052991.html )
>> on Ubuntu 18.04 machines (x86_64 & aarch64) with glibc=2.26.1 and build is
>> OK.
>>
>> There's a small issue within the following code in
>> src/java.base/unix/native/libnio/fs/UnixNativeDispatcher.c
>> Unlike readdir_r(), readdir() does not return int value. The value of res
>> is always 0 before #ifdef.
>>
>>   727     /* EINTR not listed as a possible error */
>>   728     errno = 0;
>>   729     ptr = readdir64(dirp);
>>   730     res = errno;
>>   731
>>   732 #ifdef _AIX
>>   733     /* On AIX, readdir() returns EBADF (i.e. '9') and sets 'result'
>> to NULL for the */
>>   734     /* directory stream end. Otherwise, 'errno' will contain the
>> error code. */
>>   735     if (res != 0) {
>>   736         res = (ptr == NULL && res == EBADF) ? 0 : errno;
>>   737     }
>>   738 #endif
>>
>> May I know that if this core-libs change going to be merged recently, or
>> more platforms needs to be explored?
>>
> I see your other mail looking to to add #pragma GCC ... to avoid using
> configure --disable-warnings-as-errors.
>
> I think it would be better to just replace the usage readdir_r usage in that
> native method. I've tested the patch below on macOS, Linux and Solaris.  The
> SAP folks maintain the AIX port and would need to test it on that platform
> as it eliminates the AIX specific code for dealing with end of stream that
> should be needed when using readdir.
>
> -Alan

I've checked AIX's 'readdir()' doc [1] and I guess you're right that
it never sets 'errno' to 'EBADF' while 'readdir_r()' may return it
(see [2]).
I missed this on my original patch.

So, your update seems fine to me even if I cannot evaluate it on AIX myself.

Thanks,
Bernard

[1] https://www.ibm.com/support/knowledgecenter/en/ssw_aix_71/com.ibm.aix.basetrf1/opendir.htm
[2] https://www.ibm.com/support/knowledgecenter/en/ssw_aix_71/com.ibm.aix.basetrf2/readdir_r.htm


> diff -r 9d62da00bf15 -r 5e67e87bd6fa
> src/java.base/unix/native/libnio/fs/UnixNativeDispatcher.c
> --- a/src/java.base/unix/native/libnio/fs/UnixNativeDispatcher.c Sat May 26
> 06:59:49 2018 +0200
> +++ b/src/java.base/unix/native/libnio/fs/UnixNativeDispatcher.c Mon Jun 25
> 14:17:03 2018 +0100
> @@ -1,5 +1,5 @@
>  /*
> - * Copyright (c) 2008, 2017, Oracle and/or its affiliates. All rights
> reserved.
> + * Copyright (c) 2008, 2018, Oracle and/or its affiliates. All rights
> reserved.
>   * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
>   *
>   * This code is free software; you can redistribute it and/or modify it
> @@ -72,7 +72,7 @@
>  #define fstat64 fstat
>  #define lstat64 lstat
>  #define dirent64 dirent
> -#define readdir64_r readdir_r
> +#define readdir64 readdir
>  #endif
>
>  #include "jni.h"
> @@ -720,41 +720,23 @@
>
>  JNIEXPORT jbyteArray JNICALL
>  Java_sun_nio_fs_UnixNativeDispatcher_readdir(JNIEnv* env, jclass this,
> jlong value) {
> -    struct dirent64* result;
> -    struct {
> -        struct dirent64 buf;
> -        char name_extra[PATH_MAX + 1 - sizeof result->d_name];
> -    } entry;
> -    struct dirent64* ptr = &entry.buf;
> -    int res;
>      DIR* dirp = jlong_to_ptr(value);
> +    struct dirent64* ptr;
>
> -    /* EINTR not listed as a possible error */
> -    /* TDB: reentrant version probably not required here */
> -    res = readdir64_r(dirp, ptr, &result);
> -
> -#ifdef _AIX
> -    /* On AIX, readdir_r() returns EBADF (i.e. '9') and sets 'result' to
> NULL for the */
> -    /* directory stream end. Otherwise, 'errno' will contain the error
> code. */
> -    if (res != 0) {
> -        res = (result == NULL && res == EBADF) ? 0 : errno;
> -    }
> -#endif
> -
> -    if (res != 0) {
> -        throwUnixException(env, res);
> +    errno = 0;
> +    ptr = readdir64(dirp);
> +    if (ptr == NULL) {
> +        if (errno != 0) {
> +            throwUnixException(env, errno);
> +        }
>          return NULL;
>      } else {
> -        if (result == NULL) {
> -            return NULL;
> -        } else {
> -            jsize len = strlen(ptr->d_name);
> -            jbyteArray bytes = (*env)->NewByteArray(env, len);
> -            if (bytes != NULL) {
> -                (*env)->SetByteArrayRegion(env, bytes, 0, len,
> (jbyte*)(ptr->d_name));
> -            }
> -            return bytes;
> +        jsize len = strlen(ptr->d_name);
> +        jbyteArray bytes = (*env)->NewByteArray(env, len);
> +        if (bytes != NULL) {
> +            (*env)->SetByteArrayRegion(env, bytes, 0, len,
> (jbyte*)(ptr->d_name));
>          }
> +        return bytes;
>      }
>  }