> On 28 Aug 2018, at 15:25, Baesken, Matthias <matthias.baes...@sap.com> wrote: > > Thanks Volki ! > Can I have a second review please ?
Reviewed. -Chris. > Best regards, Matthias > >> -----Original Message----- >> From: Volker Simonis <volker.simo...@gmail.com> >> Sent: Dienstag, 28. August 2018 14:07 >> To: Baesken, Matthias <matthias.baes...@sap.com> >> Cc: net-dev <net-dev@openjdk.java.net>; Chris Hegarty >> <chris.hega...@oracle.com>; Brian Burkhalter >> <brian.burkhal...@oracle.com> >> Subject: Re: [XS] RFR: 8209994: windows: >> Java_java_net_NetworkInterface_getAll misses releasing interface-list >> >> Thanks for updating the change. >> >> Looks good now! >> >> Regards, >> Volker >> On Tue, Aug 28, 2018 at 9:19 AM Baesken, Matthias >> <matthias.baes...@sap.com> wrote: >>> >>>> >>>> the change looks good but I think you should also initialize 'ifList' >>>> in 'Java_java_net_NetworkInterface_getAll()' with NULL otherwise its >>>> value is undefined and if 'enumInterfaces()' returns with an error >>>> without assigning 'ifList' you may end up calling 'free_netif()' with >>>> an undefined, non-NULL value. >>>> >>> >>> >>> Hi Volki, >>> looking at the coding, your sceanario ***should*** not happen ; >> however to be on the safe side it is for sure better to do the initialization >>> you propose. >>> >>> Looking a bit more at the coding, there is a >> Java_java_net_NetworkInterface_getAll_XP that has similar issues (missing >> free_netif calls in case of "early" returns ). >>> I adjusted this as well in the second webrev : >>> >>> http://cr.openjdk.java.net/~mbaesken/webrevs/8209994.1/ >>> >>> >>> Best regards , Matthias >>> >>> >>>> -----Original Message----- >>>> From: Volker Simonis <volker.simo...@gmail.com> >>>> Sent: Montag, 27. August 2018 17:38 >>>> To: Baesken, Matthias <matthias.baes...@sap.com> >>>> Cc: net-dev <net-dev@openjdk.java.net> >>>> Subject: Re: [XS] RFR: 8209994: windows: >>>> Java_java_net_NetworkInterface_getAll misses releasing interface-list >>>> >>>> Hi Matthias, >>>> >>>> the change looks good but I think you should also initialize 'ifList' >>>> in 'Java_java_net_NetworkInterface_getAll()' with NULL otherwise its >>>> value is undefined and if 'enumInterfaces()' returns with an error >>>> without assigning 'ifList' you may end up calling 'free_netif()' with >>>> an undefined, non-NULL value. >>>> >>>> Best regards, >>>> Volker >>>> >>>> On Mon, Aug 27, 2018 at 5:13 PM Baesken, Matthias >>>> <matthias.baes...@sap.com> wrote: >>>>> >>>>> Hello, please review this small fix ; >>>>> >>>>> >>>>> >>>>> When returning from Java_java_net_NetworkInterface_getAll >> (windows >>>> version), we have to free resources to avoid leaks. >>>>> >>>>> In some special cases this is not done . >>>>> >>>>> >>>>> >>>>> >>>>> >>>>> Bug : >>>>> >>>>> >>>>> >>>>> https://bugs.openjdk.java.net/browse/JDK-8209994 >>>>> >>>>> >>>>> >>>>> change : >>>>> >>>>> >>>>> >>>>> http://cr.openjdk.java.net/~mbaesken/webrevs/8209994/ >>>>> >>>>> >>>>> >>>>> >>>>> >>>>> >>>>> >>>>> Thanks, Matthias