This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [RFAv3 1/2] Fix leak of identifier in macro definition.

On 2019-01-26 17:31, Philippe Waroquiers wrote:

Valgrind detects leaks like the following (gdb.base/macscp.exp).
This patch fixes 1 of the 3 leaks (the last one in the list below).

The remaining leaks are better fixed in splay_tree_remove  and
splay_tree_insert in libiberty.
Tested on debian/amd64, natively and under valgrind.

==22285== 64 (48 direct, 16 indirect) bytes in 1 blocks are definitely
lost in loss record 737 of 3,377
==22285==    at 0x4C2BE6D: malloc (vg_replace_malloc.c:309)
==22285==    by 0x4049E7: xmalloc (common-utils.c:44)
==22285==    by 0x533A20: new_macro_key(macro_table*, char const*,
macro_source_file*, int) (macrotab.c:355)
==22285==    by 0x53438B: macro_define_function(macro_source_file*,
int, char const*, int, char const**, char const*) (macrotab.c:822)
==22285==    by 0x52F945: macro_define_command(char const*, int)
(macrocmd.c:409)
...
==22285== 128 (96 direct, 32 indirect) bytes in 2 blocks are
definitely lost in loss record 1,083 of 3,377
==22285==    at 0x4C2BE6D: malloc (vg_replace_malloc.c:309)
==22285==    by 0x4049E7: xmalloc (common-utils.c:44)
==22285==    by 0x533A20: new_macro_key(macro_table*, char const*,
macro_source_file*, int) (macrotab.c:355)
==22285==    by 0x534277:
macro_define_object_internal(macro_source_file*, int, char const*,
char const*, macro_special_kind) (macrotab.c:776)
==22285==    by 0x52F7E0: macro_define_command(char const*, int)
(macrocmd.c:414)
...
==22285== 177 bytes in 19 blocks are definitely lost in loss record
1,193 of 3,377
==22285==    at 0x4C2BE6D: malloc (vg_replace_malloc.c:309)
==22285==    by 0x4049E7: xmalloc (common-utils.c:44)
==22285== by 0x52F5BD: extract_identifier(char const**, int) (macrocmd.c:316) 
==22285==    by 0x52F77D: macro_define_command(char const*, int)
(macrocmd.c:355)

This is the second version of the patch.

Compared to first version, the changes are:
  * Handled the comment of Simon to have extract_identifier returning
    a unique_ptr.
  * Handled the comment of Tom that suggested rather to fix one of the
    leaks in splay-tree.c (which is a libiberty patch).


Thanks, this LGTM with a small nit to fix:
@@ -419,7 +419,7 @@ macro_define_command (const char *exp, int from_tty) 
 static void
 macro_undef_command (const char *exp, int from_tty)
 {
-  char *name;
+  gdb::unique_xmalloc_ptr<char> name;

   if (!exp)
     error (_("usage: macro undef NAME"));
@@ -428,8 +428,7 @@ macro_undef_command (const char *exp, int from_tty)
   name = extract_identifier (&exp, 0);
Declare name when assigning it, to avoid constructing an empty object and then assigning it. 


   if (! name)


Could you fix this to "name == NULL" or "name == nullptr" while at it?

Thanks!

Simon
Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]