Looks good. Please add a noreg-* label to the JIRA issue.

Naoto

On 9/27/18 7:42 AM, Baesken, Matthias wrote:
Hi,  thanks for your comments, I posted a second webrev  :

http://cr.openjdk.java.net/~mbaesken/webrevs/8211149.1/webrev/

Best regards, Matthias


-----Original Message-----
From: naoto.s...@oracle.com <naoto.s...@oracle.com>
Sent: Mittwoch, 26. September 2018 18:49
To: Baesken, Matthias <matthias.baes...@sap.com>; core-libs-
d...@openjdk.java.net
Subject: Re: [XS] RFR : 8211149: fix potential memleak in
getJavaIDFromLangID after failing SetupI18nProps call [windows]

Hi Matthias,

Thank you for fixing this. Here are my comments:

- You could merge the similar for loop at 191-193, and place the loop at
the very end before it returns.

- Please expand the if block body at line 196.

Naoto

On 9/26/18 7:29 AM, Baesken, Matthias wrote:
Hello,   could you please review this small change   (windows only)   ?

Currently, the function   "getJavaIDFromLangID"    (located in windows
java_props_md.c)
only does proper deallocations after a  successful call to the  function
SetupI18nProps.  See

      if (SetupI18nProps(MAKELCID(langID, SORT_DEFAULT),
                     &(elems[0]), &(elems[1]), &(elems[2]), &(elems[3]),
&(elems[4]))) {

     ......
          for (index = 0; index < 5; index++) {
              free(elems[index]);
          }


However a failing call (SetupI18nProps returning false) might still need
deallocations,  because the function  SetupI18nProps can malloc memory in
the failing case as well .
The change initializes   the   pointers  in  char * elems[5];
And later  frees them in case they are not NULL .

Webrev and bug :


http://cr.openjdk.java.net/~mbaesken/webrevs/8211149.0/8211149.0/webr
ev/

https://bugs.openjdk.java.net/browse/JDK-8211149


Best regards, Matthias

Reply via email to