(original) (raw)

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@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/

Thanks!
John Zavgren

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