RFR: JDK-8007606

[Vitaly Davidovich]

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> 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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/net-dev/attachments/20130205/2d62f857/attachment.html