Improve timezone mapping for AIX platform

[Masayoshi Okutsu]

Hi Volker,

You are right. The `tz' value needs to be given to 
getPlatformTimeZoneID() or the getenv() call should be moved to the 
function. Your mapPlatformToJavaTimzone(const char* tz) sounds good to me.

In any case TimeZone_md.c has become too messy by accumulating 
platform/release-specific code. I guess it's time to clean up...

Thanks,
Masayoshi

On 3/27/2014 2:48 AM, Volker Simonis wrote:
> Hi Jonathan,
>
> thanks for doing this change. Please find some comments below:
>
> 1. please update the copyright year to 2014 in every file you touch
>
> 2. in CopyFiles.gmk you can simplify the change by joining the windows
> and aix cases because on Windows OPENJDK_TARGET_OS is the same like
> OPENJDK_TARGET_OS_API_DIR. So you can write:
>
> ifneq ($(findstring $(OPENJDK_TARGET_OS), windows aix), )
>
>    TZMAPPINGS_SRC := $(JDK_TOPDIR)/src/$(OPENJDK_TARGET_OS)/lib
>
>    $(LIBDIR)/tzmappings: $(TZMAPPINGS_SRC)/tzmappings
>      $(call install-file)
>
>    COPY_FILES += $(LIBDIR)/tzmappings
>
> endif
>
> 3. regarding the changes proposed by Masayoshi: I agree that we should
> put the AIX timezone mapping code in its own function, but I don't
> think that getPlatformTimeZoneID() would be the right place. As far as
> I understand, getPlatformTimeZoneID() is used to get a platform time
> zone id if the environment variable "TZ" is not set. I don't know a
> way how this could be done on AIX (@Jonathan: maybe you know?). If
> there would be a way to get the time zone from some system files or
> so, then we should put this code into the AIX version of
> getPlatformTimeZoneID().
>
> But the current AIX code contributed by Jonathan, actually uses the
> time zone id from the "TZ" environment variable and just maps it to a
> Java compatible time zone id. So I'd suggest to refactor this code
> into a function like for example "static const char*
> mapPlatformToJavaTimzone(const char* tz)" and call that from
> findJavaTZ_md(). I think that would make the code easier to
> understand.
>
> @Masayoshi: what do you think, would you agree with such a solution.
>
> 4. I agree with Masayoshi that you should use the existing getGMTOffsetID()
>
> 5. Please notice that your change breaks all Unix builds except AIX because of:
>
>   759     }
>   760 tzerr:
>   761     if (javatz == NULL) {
>   762         time_t offset;
> ...
>   782     }
>   783 #endif
>
> I think that should read:
>
>   759
>   760     tzerr:
>   761         if (javatz == NULL) {
>   762             time_t offset;
> ...
>   782         }
>   783 #endif
>   784     }
>
> Refactoring the code in an extra function will make that error more
> evident anyway.
>
> But please always build at least on one different platform (i.e.
> Linux) to prevent such errors in the future.
>
> Regards,
> Volker
>
>
> On Wed, Mar 26, 2014 at 10:27 AM, Masayoshi Okutsu
> <masayoshi.okutsu at oracle.com> wrote:
>> Hi Jonathan,
>>
>> The AIX specific code looks OK to me. But I'd suggest the code be moved to
>> getPlatformTimeZoneID() for AIX, which just returns NULL currently. Also
>> there's a function for producing a fallback ID in "GMT±hh:mm",
>> getGMTOffsetID() which can be called in tzerr.
>>
>> Thanks,
>> Masayoshi
>>
>>
>> On 3/26/2014 3:47 PM, Jonathan Lu wrote:
>>> Hi ppc-aix-port-dev, core-libs-dev,
>>>
>>> Here's a patch for JDK-8034220,
>>>
>>> http://cr.openjdk.java.net/~luchsh/JDK-8034220/
>>>
>>> It is trying to add the a more complete timezone mapping mechanism for AIX
>>> platform.
>>> the changes are primarily based on IBM's commercial JDK code, which
>>> includes:
>>>
>>> - A new timezone mapping file added under directory jdk/src/aix/lib/
>>> - Code to parse above config file, changed file is
>>> src/solaris/native/java/util/TimeZone_md.c
>>> - And also makefile change in make/CopyFiles.gmk to copy the config file
>>>
>>> Could you pls help to review and give comments ?
>>>
>>> Cheers
>>> - Jonathan
>>