(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