On Fri, Mar 11, 2022 at 4:54 PM HAGIO KAZUHITO(萩尾 一仁) <k-hagio-ab@xxxxxxx> wrote:
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.
OK, let's leave it there. The other changes look good to me. Thanks.
For the patchset:
Acked-by: Lianbo Jiang <lijiang@xxxxxxxxxx>
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
>
-- Crash-utility mailing list Crash-utility@xxxxxxxxxx https://listman.redhat.com/mailman/listinfo/crash-utility Contribution Guidelines: https://github.com/crash-utility/crash/wiki
