Hi Alan, On 28 June 2018 at 13:49, Alan Bateman <alan.bate...@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; > } > }
