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 + #define stat stat64 #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.burkhal...@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.burkhal...@oracle.com> > wrote: > >> Hi Bernard, >> >> On Aug 14, 2018, at 4:13 AM, B. Blaser <bsr...@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 >
