RFR: JDK-8007606 (original) (raw)

John Zavgren john.zavgren at oracle.com
Fri Feb 8 09:27:06 PST 2013

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 = (MIBIFROW *) malloc(sizeof(MIBIFROW)); 163 if(ret == NULL) 164 return NULL; tableP is left dangling if line 164 runs, no? 222 if (ret != NOERROR) { 223 if (tableP != NULL) { 224 free(tableP); 225 JNUThrowByName(env, "java/lang/Error", 226 "IP Helper Library GetIfTable function failed"); 227 JNUThrowOutOfMemoryError(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(CPOEMCP, 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(wchart))+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 NetworkInterfacewinXP.c: 154 if(ret == NULL) { 155 JNUThrowByName(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

More information about the net-dev mailing list