RFR: JDK-8007606

[John Zavgren]

Greetings:
I fixed numerous errors that were identified by Mr. Davidovich.

I've posed a new webrev image at: 
http://cr.openjdk.java.net/~jzavgren/8007606/webrev.02/ 
<http://cr.openjdk.java.net/%7Ejzavgren/8007606/webrev.02/>

Thanks!
John Zavgren
On 02/05/2013 07:33 PM, Vitaly Davidovich wrote:
>
> Hi John,
>
> In NetworkInterface.c:
>
> 162           ret = (MIB_IFROW *) malloc(sizeof(MIB_IFROW));
> 163           if(ret == NULL)
> 164               return NULL;
>
> tableP is left dangling if line 164 runs, no?
>
> 222     if (ret != NO_ERROR) {
> 223         if (tableP != NULL) {
> 224             free(tableP);
>
> 225             JNU_ThrowByName(env, "java/lang/Error",
> 226                      "IP Helper Library GetIfTable function failed");
> 227             JNU_ThrowOutOfMemoryError(env, "Native heap allocation 
> failure");
> 228             return -1;
> 229         }
>
> How do you know it's an OOM here?
>
> 289         curr = (netif *)calloc(1, sizeof(netif));
> 290         if (curr != NULL) {
> 291             wlen = MultiByteToWideChar(CP_OEMCP, 0, ifrowP->bDescr,
> 292                        ifrowP->dwDescrLen, NULL, 0);
> 293             if(wlen == 0) {
> 294                 // MultiByteToWideChar should not fail
> 295                 // But in rare case it fails, we allow 'char' to 
> be displayed
> 296                 curr->displayName = (char 
> *)malloc(ifrowP->dwDescrLen + 1);
> 297                 if(curr->displayName == NULL)
> 298                     return -1;
> 299
> 300             } else {
> 301                 curr->displayName = (char 
> *)malloc(wlen*(sizeof(wchar_t))+1);
> 302                 if(curr->displayName == NULL)
> 303                     return -1;
> 304             }
>
> Before returning, doesn't curr itself need to be freed? Same thing in 
> this block:
>
> 308             if (curr->name == NULL || curr->displayName == NULL) {
> 309                 if (curr->name) free(curr->name);
> 310                 if (curr->displayName) free(curr->displayName);
> 311                 curr = NULL;
> 312             }
>
> In NetworkInterface_winXP.c:
>
> 154         if(ret == NULL) {
> 155             JNU_ThrowByName(env, "java/lang/OutOfMemoryError", 0);
> 156             return NULL;
> 157         }
>
> adapterInfo needs free here?
>
> I also noticed some ptrs are compared against NULL while others 
> against 0 - why the inconsistency?
>
> Also, might it be cleaner to use multiple chained goto's to handle 
> cleanup in functions with multiple allocations? The early returns in 
> some of these places are a recipe for leaks I think.  I'm thinking 
> something like:
>
> r1 = malloc(...);
> if (r1 == NULL) goto out1;
>
> r2 = malloc(...);
> if (r2 == NULL) goto out2;
>
> out2:
>     free(r2);
> out1:
>     free(r1);
>
> Thanks
>
> Sent from my phone
>
> On Feb 5, 2013 6:42 PM, "John Zavgren" <john.zavgren at oracle.com 
> <mailto:john.zavgren at oracle.com>> wrote:
>
>     Greetings:
>     I modified the use of malloc() and realloc() so that return values
>     are checked and potential memory leaks and data corruption
>     problems are prevented.
>
>     http://cr.openjdk.java.net/~jzavgren/8007606/webrev.01/
>     <http://cr.openjdk.java.net/%7Ejzavgren/8007606/webrev.01/>
>
>     Thanks!
>     John Zavgren
>


-- 
John Zavgren
john.zavgren at oracle.com
603-821-0904
US-Burlington-MA

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/net-dev/attachments/20130208/24ba5b3f/attachment.html