RE: [12] (AIX) 8207744: Clean up inconsistent use of opendir/closedir versus opendir64/closedir64
Hi Brian , 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 opendir64 subroutine must always be closed with the closedir64 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... Best regards, Matthias From: Brian Burkhalter <brian.burkhalter@oracle.com> Sent: Mittwoch, 1. August 2018 16:53 To: Baesken, Matthias <matthias.baesken@sap.com> Cc: Langer, Christoph <christoph.langer@sap.com> Subject: Re: [12] (AIX) 8207744: Clean up inconsistent use of opendir/closedir versus opendir64/closedir64 Hi Matthias On Aug 1, 2018, at 3:51 AM, Baesken, Matthias <matthias.baesken@sap.com<mailto:matthias.baesken@sap.com>> wrote: [F]irst sorry for the messed up previous mail . I forgot to fix the subject and remove some parts of the digest ☹ . No worries. Hi Brian , I'll build it on AIX + in case it builds fine put it into our test patch queue . Build was fine on AIX . I put it into our test patch queue and come back to your tomorrow with some info about the tests . Thank you very much! Best regards, Brian PS I will still need an imprimatur from a JDK 12 Reviewer.
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> 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 went back through all this code and looked at the IBM documentation you sent and man pages on Linux, macOS, and Solaris trying to get some consistency. This is what I came up with: http://cr.openjdk.java.net/~bpb/8207744/webrev.02/ It built successfully on our usual Linux, macOS, and Solaris targets. The Windows build is still running and tests on the other systems have begun. We’ll see how it turns out. It is not unlikely that there is some historical stuff here that I am ignorant of or something specific to these functions which this change would break, so it would be good to hear from someone better versed or with some historical context. In particular the last comment in [1] by Martin (echoed in lines 105-106 of [2]) would be worth hearing about. I don’t see that the function readdir64() still even exists on Solaris. Thanks, Brian [1] https://bugs.openjdk.java.net/browse/JDK-6339493 [2] http://cr.openjdk.java.net/~bpb/8207744/webrev.02/src/java.base/unix/native/... On Aug 2, 2018, at 7:59 AM, Brian Burkhalter <brian.burkhalter@oracle.com> wrote:
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> 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 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, No, I don’t know either whether this matters in reality but I will check it out. As long as I am fooling around in this area we might as well try to get it clean. Thanks, Brian On Aug 2, 2018, at 11:57 PM, Baesken, Matthias <matthias.baesken@sap.com> wrote:
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” ….
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 Brian, 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 ? Best regards , Matthias From: Brian Burkhalter <brian.burkhalter@oracle.com> Sent: Samstag, 4. August 2018 00:08 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>; Lindenmaier, Goetz <goetz.lindenmaier@sap.com> Subject: Re: [12] (AIX) 8207744: Clean up inconsistent use of opendir/closedir versus opendir64/closedir64 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<mailto: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 ?
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 ?
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 ?
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:
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:
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
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
Hi, Brian The change looks fine to me. From my search, I believe all usages have been covered. -Brent On 8/24/18 11:01 AM, Brian Burkhalter 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@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 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.burkhalter@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@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
Hi Volker, On Aug 27, 2018, at 6:53 AM, Volker Simonis <volker.simonis@gmail.com> wrote:
sorry, I'm a little late in the game. I've just looked at your change and in general it looks good!
Thanks: better late than never!
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)
Thanks for pointing that out. An updated webrev with this change is at http://cr.openjdk.java.net/~bpb/8207744/webrev.05/
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.
I ran this revision through our test system and there were no failures on the platforms we usually test. If your tests succeed then this should be good to go unless there are objections. Thanks, Brian
On Tue, Aug 28, 2018 at 2:26 AM Brian Burkhalter <brian.burkhalter@oracle.com> wrote:
Hi Volker,
On Aug 27, 2018, at 6:53 AM, Volker Simonis <volker.simonis@gmail.com> wrote:
sorry, I'm a little late in the game. I've just looked at your change and in general it looks good!
Thanks: better late than never!
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)
Thanks for pointing that out. An updated webrev with this change is at
http://cr.openjdk.java.net/~bpb/8207744/webrev.05/
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.
I ran this revision through our test system and there were no failures on the platforms we usually test. If your tests succeed then this should be good to go unless there are objections.
Our internal testing was successful as well. Ready to push! Regards, Volker
Thanks,
Brian
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!
participants (5)
-
B. Blaser
-
Baesken, Matthias
-
Brent Christian
-
Brian Burkhalter
-
Volker Simonis