On Fri, Mar 11, 2022 at 4:54 PM HAGIO KAZUHITO(萩尾 一仁) <[email protected]>
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 <[email protected]>
> 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
[email protected]
https://listman.redhat.com/mailman/listinfo/crash-utility
Contribution Guidelines: https://github.com/crash-utility/crash/wiki
