[Crash-utility] [PATCH 1/2] Fix memory leak in __sbitmap_for_each_set function

[lijiang]

Thank you for the fix, Sergey.

On Wed, Mar 9, 2022 at 8:00 PM <crash-utility-request at redhat.com> wrote:

> Date: Tue, 8 Mar 2022 23:27:09 +0300
> From: Sergey Samoylenko <s.samoylenko at yadro.com>
> To: <crash-utility at redhat.com>
> Cc: <linux at yadro.com>, Sergey Samoylenko <s.samoylenko at yadro.com>
> Subject: [Crash-utility] [PATCH 1/2] Fix memory leak in
>         __sbitmap_for_each_set function
> Message-ID: <20220308202710.46668-2-s.samoylenko at yadro.com>
> Content-Type: text/plain
>
> Signed-off-by: Sergey Samoylenko <s.samoylenko at yadro.com>
> ---
>  sbitmap.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/sbitmap.c b/sbitmap.c
> index 91a5274..4eaa0cc 100644
> --- a/sbitmap.c
> +++ b/sbitmap.c
>

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

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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/crash-utility/attachments/20220311/80a0bc89/attachment.htm>