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

[HAGIO KAZUHITO(萩尾 一仁)]

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
>