]> git.ipfire.org Git - thirdparty/binutils-gdb.git/commitdiff
gdb/python/guile: remove some explicit calls to xmalloc
authorAndrew Burgess <aburgess@redhat.com>
Wed, 4 Jun 2025 18:54:01 +0000 (19:54 +0100)
committerAndrew Burgess <aburgess@redhat.com>
Fri, 6 Jun 2025 22:46:47 +0000 (23:46 +0100)
In gdbpy_parse_command_name (python/py-cmd.c) there is a call to
xmalloc that can easily be replaced with a call to
make_unique_xstrndup, which makes the code easier to read (I think).

In gdbscm_parse_command_name (guile/scm-cmd.c) the same fix can be
applied to remove an identical xmalloc call.  And there is an
additional xmalloc call, which can also be replaced with
make_unique_xstrndup in the same way.

The second xmalloc call in gdbscm_parse_command_name was also present
in gdbpy_parse_command_name at one point, but was replaced with a use
of std::string by this commit:

  commit 075c55e0cc0a68eeab777027213c2f545618e844
  Date:   Wed Dec 26 11:05:57 2018 -0700

      Remove more calls to xfree from Python

I haven't changed the gdbscm_parse_command_name to use std::string
though, as that doesn't work well with the guile exception model.
Guile exceptions work by performing a longjmp from the function that
raises the exception, back to the guile run-time.  The consequence of
this is that destructors are not run.  For example, if
gdbscm_parse_command_name calls gdbscm_out_of_range_error, then any
function local objects in gdbscm_parse_command_name will not have
their destructors called.

What this means is that, for the existing `result` and `prefix_text`
locals, any allocated memory managed by these objects will be leaked
if an exception is called.  However, fixing this is pretty easy, one
way is to just assign nullptr to these locals before raising the
exception, this would cause the allocated memory to be released.

But for std::string it is harder to ensure that the managed memory has
actually been released.  We can call std::string::clear() and then
maybe std::string::shrink_to_fit(), but this is still not guaranteed
to release any managed memory.  In fact, I believe the only way to
ensure all managed memory is released, is to call the std::string
destructor.

And so, for functions that can throw a guile exception, it is easier
to just avoid std::string.

As for the memory leak that I identify above; I'll fix that in a
follow on commit.

Approved-By: Tom Tromey <tom@tromey.com>
gdb/guile/scm-cmd.c
gdb/python/py-cmd.c

index 74646f35ea2618a470517396d67bf4830171d333..46825f96b9e63e878e6206ce87d43cb72f4153de 100644 (file)
@@ -496,9 +496,9 @@ gdbscm_parse_command_name (const char *name,
   /* Find first character of the final word.  */
   for (; i > 0 && valid_cmd_char_p (name[i - 1]); --i)
     ;
-  gdb::unique_xmalloc_ptr<char> result ((char *) xmalloc (lastchar - i + 2));
-  memcpy (result.get (), &name[i], lastchar - i + 1);
-  result.get ()[lastchar - i + 1] = '\0';
+
+  gdb::unique_xmalloc_ptr<char> result
+    = make_unique_xstrndup (&name[i], lastchar - i + 1);
 
   /* Skip whitespace again.  */
   for (--i; i >= 0 && (name[i] == ' ' || name[i] == '\t'); --i)
@@ -509,9 +509,8 @@ gdbscm_parse_command_name (const char *name,
       return result.release ();
     }
 
-  gdb::unique_xmalloc_ptr<char> prefix_text ((char *) xmalloc (i + 2));
-  memcpy (prefix_text.get (), name, i + 1);
-  prefix_text.get ()[i + 1] = '\0';
+  gdb::unique_xmalloc_ptr<char> prefix_text
+    = make_unique_xstrndup (name, i + 1);
 
   const char *prefix_text2 = prefix_text.get ();
   elt = lookup_cmd_1 (&prefix_text2, *start_list, NULL, NULL, 1);
index e8c29042430b3b907bbe1a205272bd199d9081c1..dc5e270005991619c5232a37cd86837d5d637dc3 100644 (file)
@@ -372,9 +372,8 @@ gdbpy_parse_command_name (const char *name,
   for (; i > 0 && valid_cmd_char_p (name[i - 1]); --i)
     ;
 
-  gdb::unique_xmalloc_ptr<char> result ((char *) xmalloc (lastchar - i + 2));
-  memcpy (result.get (), &name[i], lastchar - i + 1);
-  result.get ()[lastchar - i + 1] = '\0';
+  gdb::unique_xmalloc_ptr<char> result
+    = make_unique_xstrndup (&name[i], lastchar - i + 1);
 
   /* Skip whitespace again.  */
   for (--i; i >= 0 && (name[i] == ' ' || name[i] == '\t'); --i)