Hi Brian , looks like the function struct dirent64 *readdir64 (DIR64 *DirectoryPointer); returns dirent64* ( according to https://www.ibm.com/support/knowledgecenter/no/ssw_aix_72/com.ibm.aix.basetr... ) and some of the files below still have dirent* on AIX ( at some places it is redefined ). For example : http://cr.openjdk.java.net/~bpb/8207744/webrev.02/src/java.base/unix/native/... 508 jint unix_getChildren(JNIEnv *env, jlong jpid, jlongArray jarray, 509 jlongArray jparentArray, jlongArray jstimesArray) { 510 DIR* dir; 511 struct dirent* ptr; Not sure if this is really an issue in "real life" .... Best regards, Matthias From: Brian Burkhalter <brian.burkhalter@oracle.com> Sent: Donnerstag, 2. August 2018 16:59 To: Baesken, Matthias <matthias.baesken@sap.com> Cc: core-libs-dev@openjdk.java.net; Langer, Christoph <christoph.langer@sap.com>; Simonis, Volker <volker.simonis@sap.com> Subject: Re: [12] (AIX) 8207744: Clean up inconsistent use of opendir/closedir versus opendir64/closedir64 Hi Matthias, Thanks for the build + test. I will take a look at what you observed. The whole process is a bit protracted since I cannot build and test but your assistance is appreciated. Thanks, Brian On Aug 2, 2018, at 5:24 AM, Baesken, Matthias <matthias.baesken@sap.com<mailto:matthias.baesken@sap.com>> wrote: builds + tests ( with your change included) last night on AIX were good . However while looking into the coding a bit more, I noticed the following : IBM states for AIX : https://www.ibm.com/support/knowledgecenter/no/ssw_aix_72/com.ibm.aix.basetr... Note An open directory by opendir64subroutine must always be closed with theclosedir64 subroutine to ensure that the next attempt to open that directory is successful. In addition, it must be operated using the 64-bit interfaces (readdir64, telldir64, seekdir64,rewinddir64, and closedir64) to obtain the correct directory information. However this is currently not the case everywhere : for example I see in http://cr.openjdk.java.net/~bpb/8207744/webrev.01/src/java.base/share/native... a readdir (without 64). Same for http://cr.openjdk.java.net/~bpb/8207744/webrev.01/raw_files/new/src/java.bas...

HI Matthias, I think I fixed the dirent problem: thanks for pointing it out. http://cr.openjdk.java.net/~bpb/8207744/webrev.03/ The usual builds passed and tests are running. I think that there is some _ALLBSD_SOURCE cruft in UnixNativeDispatcher.c which could be cleaned up but we’ll leave that for another day. Thanks, Brian On Aug 2, 2018, at 11:57 PM, Baesken, Matthias <matthias.baesken@sap.com> wrote:

looks like the function

struct dirent64 *readdir64 (DIR64 *DirectoryPointer);

returns dirent64* ( according to https://www.ibm.com/support/knowledgecenter/no/ssw_aix_72/com.ibm.aix.basetr... )

and some of the files below still have dirent* on AIX ( at some places it is redefined ).

For example :

http://cr.openjdk.java.net/~bpb/8207744/webrev.02/src/java.base/unix/native/...

508 jint unix_getChildren(JNIEnv *env, jlong jpid, jlongArray jarray, 509 jlongArray jparentArray, jlongArray jstimesArray) { 510 DIR* dir; 511 struct dirent* ptr;

Not sure if this is really an issue in “real life” ….

Hi Matthias, Yes, that was intentional. From the documentation it looks as if the Linux / Solaris *64 functions in question are now legacy so I changed them for hopefully better consistency. I did not see any problems in building or testing with these changes. If someone knows these changes to be incorrect then I would appreciate being so informed. Does this latest (.03) patch check out on AIX? Thanks, Brian On Aug 7, 2018, at 3:12 AM, Baesken, Matthias <matthias.baesken@sap.com> wrote:

small remark from my side , looks like you changed in

src/java.base/unix/native/libjava/TimeZone_md.c

src/java.base/unix/native/libjava/childproc.c

and

src/jdk.management/unix/native/libmanagement_ext/OperatingSystemImpl.c

at some places dirent64 / readdir64 to dirent / readdir in the linux/solaris coding.

for example

TimeZone_md.c

122 static char * 123 findZoneinfoFile(char *buf, size_t size, const char *dir) 124 { 125 DIR *dirp = NULL; 126 struct stat statbuf; 127 struct dirent *dp = NULL;.

Was it intended to change for linux/solaris ?

Hi Brian, Among the files you suggest to fix, only the following ones are still using 'readdir64' for other systems than AIX: http://cr.openjdk.java.net/~bpb/8207744/webrev.03/src/java.base/unix/native/... http://cr.openjdk.java.net/~bpb/8207744/webrev.03/src/java.base/unix/native/... I think you could also use 'readdir' and remove lines like the following ones in both files: 67 #if defined(_ALLBSD_SOURCE) 68 #define dirent64 dirent 69 #define readdir64 readdir 70 #define stat64 stat The following file is the only one that uses 'readdir64' on BSD which is suspect: http://cr.openjdk.java.net/~bpb/8207744/webrev.03/src/jdk.management/unix/na... I guess you could simply remove the following lines: 77 #if defined(_ALLBSD_SOURCE) 78 #define DIR DIR64 79 #define dirent dirent64 80 #define opendir opendir64 81 #define closedir closedir64 82 #define readdir readdir64 83 #endif In the following file: http://cr.openjdk.java.net/~bpb/8207744/webrev.03/src/java.base/unix/native/... ... 'readdir' seems not to be used on AIX, see: 81 #if defined(__linux__) || defined(MACOSX) || defined(__solaris__) So, some lines following '53 #if defined(_AIX) ...' are probably not necessary. Finally, referring to [1], I note that 'readdir64' is only necessary on Linux to access large file systems using 32-bit architectures: "To support large filesystems on 32-bit machines there are LFS variants of the last two functions." [...] So, removing it is OK if this feature isn't required by the JDK. Cheers, Bernard [1] https://www.gnu.org/software/libc/manual/html_node/Reading_002fClosing-Direc... On 9 August 2018 at 22:56, Brian Burkhalter <brian.burkhalter@oracle.com> wrote:

Hello,

The latest build + test runs with the .03 revision of this patch check out on AIX, Linux-x64, macOS, Solaris-sparc, and Windows-x64.

If there is a Reviewer available who approves of these changes then please do so.

Thanks,

Brian

On Aug 7, 2018, at 3:41 PM, Brian Burkhalter <brian.burkhalter@oracle.com> wrote:

...

Hi Matthias,

Yes, that was intentional. From the documentation it looks as if the Linux / Solaris *64 functions in question are now legacy so I changed them for hopefully better consistency. I did not see any problems in building or testing with these changes. If someone knows these changes to be incorrect then I would appreciate being so informed.

Does this latest (.03) patch check out on AIX?

Thanks,

Brian

On Aug 7, 2018, at 3:12 AM, Baesken, Matthias <matthias.baesken@sap.com> wrote:

...

small remark from my side , looks like you changed in

src/java.base/unix/native/libjava/TimeZone_md.c

src/java.base/unix/native/libjava/childproc.c

and

src/jdk.management/unix/native/libmanagement_ext/OperatingSystemImpl.c

at some places dirent64 / readdir64 to dirent / readdir in the linux/solaris coding.

for example

TimeZone_md.c

122 static char * 123 findZoneinfoFile(char *buf, size_t size, const char *dir) 124 { 125 DIR *dirp = NULL; 126 struct stat statbuf; 127 struct dirent *dp = NULL;.

Was it intended to change for linux/solaris ?

Seems quite good to me, last notes: 1) dealing with 'stat/stat64' in 'UnixFileSystem_md.c' might be outside the scope of this fix (?) even if fully pertinent per [1]. In the same file, I think '#define dirent dirent64' is probably missing for AIX. 2) I guess '#if defined(_AIX) ...' is now missing in 'OperatingSystemImpl.c': #if defined(_AIX) #define DIR DIR64 #define dirent dirent64 #define opendir opendir64 #define readdir readdir64 #define closedir closedir64 #endif You'll probably need some more reviews especially for other systems than Linux 64-bit. Thanks, Bernard [1] https://www.gnu.org/software/libc/manual/html_node/Reading-Attributes.html On 13 August 2018 at 23:26, Brian Burkhalter <brian.burkhalter@oracle.com> wrote:

Hi Bernard,

I updated the patch per your suggestions and it checks out on our systems.

http://cr.openjdk.java.net/~bpb/8207744/webrev.04/

Thanks,

Brian

On Aug 10, 2018, at 6:20 AM, B. Blaser <bsrbnd@gmail.com> wrote:

Among the files you suggest to fix, only the following ones are still using 'readdir64' for other systems than AIX:

This one could still use a Reviewer approval or rejection. Thanks, Brian On Aug 14, 2018, at 1:34 PM, Brian Burkhalter <brian.burkhalter@oracle.com> wrote:

Hi Bernard,

On Aug 14, 2018, at 4:13 AM, B. Blaser <bsrbnd@gmail.com> wrote:

...

Seems quite good to me, last notes:

1) dealing with 'stat/stat64' in 'UnixFileSystem_md.c' might be outside the scope of this fix (?) even if fully pertinent per [1].

It might be slightly out of scope but I think it’s OK as stat64 was defined inside an #if defined(_ALLBSD_SOURCE) conditional compilation block.

...

In the same file, I think '#define dirent dirent64' is probably missing for AIX.

Fixed.

...

2) I guess '#if defined(_AIX) ...' is now missing in 'OperatingSystemImpl.c': #if defined(_AIX) #define DIR DIR64 #define dirent dirent64 #define opendir opendir64 #define readdir readdir64 #define closedir closedir64 #endif

Fixed.

Webrev updated in place: http://cr.openjdk.java.net/~bpb/8207744/webrev.04/.

Checks out on Linux-x64, macOS, Solaris-sparcv9, Windows-x64.

...

You'll probably need some more reviews especially for other systems than Linux 64-bit.

It would not hurt. In any case I do not yet have a Reviewer approval.

Thanks,

Brian

Thanks, Brent! Brian On Aug 24, 2018, at 12:14 PM, Brent Christian <brent.christian@oracle.com> wrote:

The change looks fine to me. From my search, I believe all usages have been covered.

Hi Volker, Thanks for the corroboration. I’ll plan to push this tomorrow. Brian On Aug 28, 2018, at 5:12 AM, Volker Simonis <volker.simonis@gmail.com> wrote:

Our internal testing was successful as well.

Ready to push!