Fix for small leak in TimeZone_md.c

[Andrew Hughes]

----- Original Message -----
> Lander,
> 
> My few cents ...
> 
> 1. After you refactoring freetz is never actually used. It might be
> better to just get rid of this variable.
> 

At present, it's still being used to hold the old version of tz:

        tz = getPlatformTimeZoneID();
        freetz = tz;

So both tz and freetz point to the return value of getPlatformTimeZoneID.
tz may then be moved forward due to:

    if (tz != NULL && *tz == ':') {
        tz++;
    }

and then it is replaced altogether:

    if (tz != NULL && strcmp(tz, "localtime") == 0) {
        tz = getSolarisDefaultZoneID();

Now tz contains the return value of getSolarisDefaultZoneID, so only
freetz references the old return value from getPlatformTimeZoneID.
This code then frees it:

        if (freetz != NULL) {
            free((void *) freetz);
        }
        freetz = tz;

and tz and freetz match again.

You could get rid of freetz if tz was freed before being overwritten
in this case, but the same isn't true in the AIX case where tz is
passed to mapPlatformToJavaTimezone.

So it would need further refactoring rather than simply being dropped.

> 2. Also you can remove javatz as well and further refactor the code to
> 
>     return (tz == NULL) ? NULL : strdup(tz);
> 

At present, tz, as referenced by freetz, is freed after the strdup.
I do wonder why it needs to be copied at all when tz itself is the
result of a strdup from the call to getPlatformTimeZoneID,
getSolarisDefaultZoneID or mapPlatformToJavaTimezone.

> -Dmitry
> 
> On 2015-08-19 16:14, Langer, Christoph wrote:
> > Hi Thomas,
> >
> > thanks for your review.
> >
> > Axel already helped me to create a bug report:
> > https://bugs.openjdk.java.net/browse/JDK-8133830
> >
> > I had also updated my webrev, as I would like to take the chance to do some
> > minor refactoring of the AIX porting code for better readability:
> > http://cr.openjdk.java.net/~asiebenborn/8133830/webrev/
> >
> > Can you review the updated change as well?
> >
> > Thanks
> > Christoph
> >
> > From: Thomas Stüfe [mailto:thomas.stuefe at gmail.com]
> > Sent: Mittwoch, 19. August 2015 13:12
> > To: Langer, Christoph
> > Cc: jdk9-dev at openjdk.java.net
> > Subject: Re: Fix for small leak in TimeZone_md.c
> >
> > Hi Christoph,
> >
> > looks fine. I opened a bug for you:
> > https://bugs.openjdk.java.net/browse/JDK-8133933
> >
> > But this needs another reviewer and a sponsor.
> >
> > Kind Regards, Thomas
> >
> >
> >
> > On Tue, Aug 18, 2015 at 11:52 AM, Langer, Christoph
> > <christoph.langer at sap.com<mailto:christoph.langer at sap.com>> wrote:
> > Hi all,
> >
> > I think there is a small memory leak in TimeZone_md.c in the case of
> > solaris "localtime".
> >
> > When getPlatformTimeZoneID() is called and its result is "localtime", the
> > returned string buffer is not freed as the pointer "freetz" is overwritten
> > after the call to getSolarisDefaultZoneID().
> >
> > Please have a look at my webrev:
> > http://cr.openjdk.java.net/~asiebenborn/christoph/webrev/ for a potential
> > fix.
> >
> > A bug was not yet created but should be done after you agree that this is
> > an issue.
> >
> > Thanks in advance for comments.
> >
> > Best regards
> > Christoph
> >
> 
> 
> --
> Dmitry Samersoff
> Oracle Java development team, Saint Petersburg, Russia
> * I would love to change the world, but they won't give me the sources.
> 
> 

-- 
Andrew :)

Senior Free Java Software Engineer
Red Hat, Inc. (http://www.redhat.com)

PGP Key: ed25519/35964222 (hkp://keys.gnupg.net)
Fingerprint = 5132 579D D154 0ED2 3E04  C5A0 CFDA 0F9B 3596 4222

PGP Key: rsa4096/248BDC07 (hkp://keys.gnupg.net)
Fingerprint = EC5A 1F5E C0AD 1D15 8F1F  8F91 3B96 A578 248B DC07