[12] (AIX) 8207744: Clean up inconsistent use of opendir/closedir versus opendir64/closedir64 (original) (raw)

Volker Simonis volker.simonis at gmail.com
Mon Aug 27 13:53:01 UTC 2018


Hi Brian,

sorry, I'm a little late in the game. I've just looked at your change and in general it looks good!

There's one thing however I think you still have to fix. After changing 'stat64' to 'stat' in UnixFileSystem_md.c you should define 'stat' to 'stat64' on AIX if you don't want to change the current behavior:

$ hg diff diff -r 8455a2fda5a8 src/java.base/unix/native/libjava/UnixFileSystem_md.c --- a/src/java.base/unix/native/libjava/UnixFileSystem_md.c Mon Aug 27 11:30:50 2018 +0200 +++ b/src/java.base/unix/native/libjava/UnixFileSystem_md.c Mon Aug 27 14:55:07 2018 +0200 @@ -59,6 +59,7 @@ #define opendir opendir64 #define readdir readdir64 #define closedir closedir64

#endif

#if defined(solaris) && !defined(NAME_MAX)

The build and first tests with this addition look good. I'll also put the fix into our nightly queue to run some more extensive tests and let you know the results tomorrow.

Thank you and best regards, Volker

On Fri, Aug 24, 2018 at 8:01 PM, Brian Burkhalter <brian.burkhalter at oracle.com> wrote:

This one could still use a Reviewer approval or rejection.

Thanks, Brian On Aug 14, 2018, at 1:34 PM, Brian Burkhalter <brian.burkhalter at oracle.com> wrote:

Hi Bernard,

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

Seems quite good to me, last notes:

1) dealing with 'stat/stat64' in 'UnixFileSystemmd.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(ALLBSDSOURCE) 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



More information about the core-libs-dev mailing list