Re: [Crash-utility] [PATCH 1/2] Fix memory leak in __sbitmap_for_each_set function
Hi Lianbo,
-----Original Message-----
This reminds me, if parse_line() or dump_struct_member() fails, is
there a potential risk of memory leaks
in the dump_struct_members()?
file: sbitmap.c
432 static void dump_struct_members(const char *s, ulong addr, unsigned radix)
433 {
434 int i, argc;
435 char *p1, *p2;
436 char *structname, *members;
437 char *arglist[MAXARGS];
438
439 structname = GETBUF(strlen(s) + 1);
440 members = GETBUF(strlen(s) + 1);
441
442 strcpy(structname, s);
443 p1 = strstr(structname, ".") + 1;
444
445 p2 = strstr(s, ".") + 1;
446 strcpy(members, p2);
447 replace_string(members, ",", ' ');
448 argc = parse_line(members, arglist);
449
450 for (i = 0; i < argc; i++) {
451 *p1 = NULLCHAR;
452 strcat(structname, arglist[i]);
453 dump_struct_member(structname, addr, radix);
454 }
455
456 FREEBUF(structname);
457 FREEBUF(members);
458 }
I noticed that the parse_line() has a return value, but the dump_struct_member() has no
return value, is
there a good way to avoid the potential risks? Or leave it there?
BTW: I saw the similar issues in tools.c
um, the fact is, all buffers that GETBUF() allocates will be freed automatically
after the last command execution in free_all_bufs():
main_loop
while (TRUE) {
process_command_line
restore_sanity
free_all_bufs <<--
exec_command
}
so to free all buffers is better coding practice and has several pros if you can,
but it's not necessary to work too hard for it.
Thanks,
Kazu
Thanks.
Lianbo
@@ -272,7 +272,7 @@ static void __sbitmap_for_each_set(const struct sbitmap_context
*sc,
if (nr >= depth)
break;
if (!fn((index << sc->shift) + nr, data))
- return;
+ goto exit;
nr++;
}
@@ -282,6 +282,7 @@ next:
index = 0;
}
+exit:
FREEBUF(sbitmap_word_buf);
}
--
2.25.1