JDK10: RFR(xxs): 8181207: 8177809 breaks AIX 5.3, 6.1 builds

[Langer, Christoph]

Hi Thomas,

looks good.

Some suggestions about formatting:

a) you could code write your code like this:

#if defined(_AIX)
…
#elif defined(MACOSX)
…
#else
…
#endif

That way the coding has 3 clear sections and you don’t have to do an #ifdef block in another #ifdef.

b)  Line 234, 235 (AIX block), rather write:
             rv   = (jlong)sb.st_mtime * 1000;
             rv += (jlong)sb.st_mtime_n / 1000000;
Then it looks aligned with the MACOSX and the default section.

Best regards
Christoph



From: ppc-aix-port-dev [mailto:ppc-aix-port-dev-bounces at openjdk.java.net] On Behalf Of Thomas Stüfe
Sent: Mittwoch, 31. Mai 2017 17:30
To: Volker Simonis <volker.simonis at gmail.com>
Cc: ppc-aix-port-dev at openjdk.java.net; Java Core Libs <core-libs-dev at openjdk.java.net>
Subject: Re: JDK10: RFR(xxs): 8181207: 8177809 breaks AIX 5.3, 6.1 builds

Hi Volker,

Good suggestions! I completely overlooked the ..._n members in stat64 struct. It seems it is even documented: https://www.ibm.com/support/knowledgecenter/ssw_aix_72/com.ibm.aix.files/stat.h.htm

new webrev: http://cr.openjdk.java.net/~stuefe/webrevs/8181207-8177809-breaks-AIX-builds/webrev.01/webrev/

..Thomas

On Wed, May 31, 2017 at 10:49 AM, Volker Simonis <volker.simonis at gmail.com<mailto:volker.simonis at gmail.com>> wrote:
Hi Thomas,

as far as I can see, AIX supports both, the st_[a,c,m]time members in
the stat64 structure for seconds and the corresponding
st_[a,c,m]time_n members for nanosecond resolution since at least 5.3.
Can you please use both - there's no reason to discriminate AIX here
:)

Also, can you please change the code such that we have:

#ifdef MACOSX
...
#else
#ifdef AIX
...
#else
...
#endif
#endif

I don't really like using "ifndef XXX" for everything else except XXX.

Thnank you and best regards,
Volker


On Tue, May 30, 2017 at 11:46 AM, Thomas Stüfe <thomas.stuefe at gmail.com<mailto:thomas.stuefe at gmail.com>> wrote:
> Hi all,
>
> may I have please a review for this tiny change:
>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8181207
> webrev:
> http://cr.openjdk.java.net/~stuefe/webrevs/8181207-8177809-breaks-AIX-builds/webrev.00/webrev/
>
> This reverts 8177809 for AIX because it leads to build errors on older AIX
> systems. We want to retain the ability to build on older AIX releases.
>
> Thanks, Thomas