Re: RFR-JDK8012108
On Apr 20, 2013, at 4:40 AM, Dmitry Samersoff wrote: > Kurchi, > > if *netaddrPP == NULL at 367 and calloc fails at 391 we would jump > to freeAllocatedMemory with start == NULL > True, but then we skip to line 444 since *netaddrPP == NULL, so we don't get to line 438. I am just saying it is not strictly necessary to check if start is null before entering the first if block. We might want to guard against how the code changes in future and put in the check though. > > I have no ideas whether this code path is possible in reality or not, > but it stops my eyes. > > -Dmitry > > > On 2013-04-20 14:55, Kurchi Subhra Hazra wrote: >> >> >> On Apr 20, 2013, at 3:16 AM, Dmitry Samersoff >> wrote: >> >>> John, >>> >>> 102, 145 else is redundant. >>> >>> 438 - we will crash here if start is null >> >> >> Maybe it is good to have an additional check for start != null, but from >> what I see, start will not be null if *netaddrPP is not null. >> >> >> >> >> >>> >>> -Dmitry >>> >>> >>> On 2013-04-20 01:33, John Zavgren wrote: Greetings: I fixed the bad realloc pattern. Please let me know what you think. http://cr.openjdk.java.net/~jzavgren/8012108/webrev.02/ Thanks! John Z - Original Message - From: chris.hega...@oracle.com To: net-dev@openjdk.java.net, john.zavg...@oracle.com Cc: dmitry.samers...@oracle.com Sent: Friday, April 19, 2013 8:59:25 AM GMT -05:00 US/Canada Eastern Subject: Re: RFR-JDK8012108 On 18/04/2013 22:11, Dmitry Samersoff wrote: > John, > > I see bad realloc pattern here. Could you fix it as well? Yes, please. Otherwise the changes look fine. -Chris. > > e.g. > > 93 adapterInfo = (IP_ADAPTER_ADDRESSES *) realloc (adapterInfo, len); > > -Dmitry > > On 2013-04-19 00:56, John Zavgren wrote: >> Greetings: >> >> I fixed a case in the windows native code where calloc() was being used >> without checking it's returned value. >> >> http://cr.openjdk.java.net/~jzavgren/8012108/webrev.01/ >> >> Thanks! >> John Zavgren >>> >>> >>> -- >>> Dmitry Samersoff >>> Oracle Java development team, Saint Petersburg, Russia >>> * Give Rabbit time, and he'll always get the answer > > > -- > Dmitry Samersoff > Oracle Java development team, Saint Petersburg, Russia > * Give Rabbit time, and he'll always get the answer
Re: RFR-JDK8012108
Kurchi, if *netaddrPP == NULL at 367 and calloc fails at 391 we would jump to freeAllocatedMemory with start == NULL I have no ideas whether this code path is possible in reality or not, but it stops my eyes. -Dmitry On 2013-04-20 14:55, Kurchi Subhra Hazra wrote: > > > On Apr 20, 2013, at 3:16 AM, Dmitry Samersoff > wrote: > >> John, >> >> 102, 145 else is redundant. >> >> 438 - we will crash here if start is null > > > Maybe it is good to have an additional check for start != null, but from what > I see, start will not be null if *netaddrPP is not null. > > > > > >> >> -Dmitry >> >> >> On 2013-04-20 01:33, John Zavgren wrote: >>> Greetings: >>> >>> I fixed the bad realloc pattern. Please let me know what you think. >>> http://cr.openjdk.java.net/~jzavgren/8012108/webrev.02/ >>> >>> Thanks! >>> John Z >>> >>> >>> - Original Message - >>> From: chris.hega...@oracle.com >>> To: net-dev@openjdk.java.net, john.zavg...@oracle.com >>> Cc: dmitry.samers...@oracle.com >>> Sent: Friday, April 19, 2013 8:59:25 AM GMT -05:00 US/Canada Eastern >>> Subject: Re: RFR-JDK8012108 >>> >>> On 18/04/2013 22:11, Dmitry Samersoff wrote: John, I see bad realloc pattern here. Could you fix it as well? >>> >>> Yes, please. Otherwise the changes look fine. >>> >>> -Chris. >>> e.g. 93 adapterInfo = (IP_ADAPTER_ADDRESSES *) realloc (adapterInfo, len); -Dmitry On 2013-04-19 00:56, John Zavgren wrote: > Greetings: > > I fixed a case in the windows native code where calloc() was being used > without checking it's returned value. > > http://cr.openjdk.java.net/~jzavgren/8012108/webrev.01/ > > Thanks! > John Zavgren >> >> >> -- >> Dmitry Samersoff >> Oracle Java development team, Saint Petersburg, Russia >> * Give Rabbit time, and he'll always get the answer -- Dmitry Samersoff Oracle Java development team, Saint Petersburg, Russia * Give Rabbit time, and he'll always get the answer
Re: RFR-JDK8012108
On Apr 20, 2013, at 3:16 AM, Dmitry Samersoff wrote: > John, > > 102, 145 else is redundant. > > 438 - we will crash here if start is null Maybe it is good to have an additional check for start != null, but from what I see, start will not be null if *netaddrPP is not null. > > -Dmitry > > > On 2013-04-20 01:33, John Zavgren wrote: >> Greetings: >> >> I fixed the bad realloc pattern. Please let me know what you think. >> http://cr.openjdk.java.net/~jzavgren/8012108/webrev.02/ >> >> Thanks! >> John Z >> >> >> - Original Message - >> From: chris.hega...@oracle.com >> To: net-dev@openjdk.java.net, john.zavg...@oracle.com >> Cc: dmitry.samers...@oracle.com >> Sent: Friday, April 19, 2013 8:59:25 AM GMT -05:00 US/Canada Eastern >> Subject: Re: RFR-JDK8012108 >> >> On 18/04/2013 22:11, Dmitry Samersoff wrote: >>> John, >>> >>> I see bad realloc pattern here. Could you fix it as well? >> >> Yes, please. Otherwise the changes look fine. >> >> -Chris. >> >>> >>> e.g. >>> >>> 93 adapterInfo = (IP_ADAPTER_ADDRESSES *) realloc (adapterInfo, len); >>> >>> -Dmitry >>> >>> On 2013-04-19 00:56, John Zavgren wrote: Greetings: I fixed a case in the windows native code where calloc() was being used without checking it's returned value. http://cr.openjdk.java.net/~jzavgren/8012108/webrev.01/ Thanks! John Zavgren > > > -- > Dmitry Samersoff > Oracle Java development team, Saint Petersburg, Russia > * Give Rabbit time, and he'll always get the answer
Re: RFR-JDK8012108
John, 102, 145 else is redundant. 438 - we will crash here if start is null -Dmitry On 2013-04-20 01:33, John Zavgren wrote: > Greetings: > > I fixed the bad realloc pattern. Please let me know what you think. > http://cr.openjdk.java.net/~jzavgren/8012108/webrev.02/ > > Thanks! > John Z > > > - Original Message - > From: chris.hega...@oracle.com > To: net-dev@openjdk.java.net, john.zavg...@oracle.com > Cc: dmitry.samers...@oracle.com > Sent: Friday, April 19, 2013 8:59:25 AM GMT -05:00 US/Canada Eastern > Subject: Re: RFR-JDK8012108 > > On 18/04/2013 22:11, Dmitry Samersoff wrote: >> John, >> >> I see bad realloc pattern here. Could you fix it as well? > > Yes, please. Otherwise the changes look fine. > > -Chris. > >> >> e.g. >> >> 93 adapterInfo = (IP_ADAPTER_ADDRESSES *) realloc (adapterInfo, len); >> >> -Dmitry >> >> On 2013-04-19 00:56, John Zavgren wrote: >>> Greetings: >>> >>> I fixed a case in the windows native code where calloc() was being used >>> without checking it's returned value. >>> >>> http://cr.openjdk.java.net/~jzavgren/8012108/webrev.01/ >>> >>> Thanks! >>> John Zavgren >> >> -- Dmitry Samersoff Oracle Java development team, Saint Petersburg, Russia * Give Rabbit time, and he'll always get the answer
Re: RFR-JDK8012108
On 04/20/2013 02:06 AM, Kurchi Subhra Hazra wrote: Hi John, Minor nit, the formatting around line 101 looks off, How about something like this: if { // remains same } else { adapterInfo = adapterInfoTemp; } Or simply, just leave the else out. adapterInfoTemp = (IP_ADAPTER_ADDRESSES *) realloc (adapterInfo, len); if (adapterInfoTemp == NULL) { free(adapterInfo); return -1; } adapterInfo = adapterInfoTemp; -Chris. - Kurchi On 4/19/13 2:33 PM, John Zavgren wrote: Greetings: I fixed the bad realloc pattern. Please let me know what you think. http://cr.openjdk.java.net/~jzavgren/8012108/webrev.02/ Thanks! John Z - Original Message - From:chris.hega...@oracle.com To:net-dev@openjdk.java.net,john.zavg...@oracle.com Cc:dmitry.samers...@oracle.com Sent: Friday, April 19, 2013 8:59:25 AM GMT -05:00 US/Canada Eastern Subject: Re: RFR-JDK8012108 On 18/04/2013 22:11, Dmitry Samersoff wrote: John, I see bad realloc pattern here. Could you fix it as well? Yes, please. Otherwise the changes look fine. -Chris. e.g. 93 adapterInfo = (IP_ADAPTER_ADDRESSES *) realloc (adapterInfo, len); -Dmitry On 2013-04-19 00:56, John Zavgren wrote: Greetings: I fixed a case in the windows native code where calloc() was being used without checking it's returned value. http://cr.openjdk.java.net/~jzavgren/8012108/webrev.01/ Thanks! John Zavgren