[3/3] gdb: make macro_stringify return a gdb::unique_xmalloc_ptr<char>

Message ID 20200628165628.96730-4-simon.marchi@polymtl.ca
State New
Headers show
Series
  • Small cleanups in macro code
Related show

Commit Message

Shahab Vahedi via Gdb-patches June 28, 2020, 4:56 p.m.
The change to macro_stringify is straightforward.  This allows removing
the manual memory management in fixup_definition.

gdb/ChangeLog:

	* macroexp.h (macro_stringify): Return
	gdb::unique_xmalloc_ptr<char>.
	* macroexp.c (macro_stringify): Likewise.
	* macrotab.c (fixup_definition): Update.

Change-Id: Id7db8988bdbd569dd51c4f4655b00eb26db277cb
---
 gdb/macroexp.c |  4 ++--
 gdb/macroexp.h |  6 ++----
 gdb/macrotab.c | 14 ++++----------
 3 files changed, 8 insertions(+), 16 deletions(-)

-- 
2.27.0

Comments

Tom Tromey June 30, 2020, 8:55 p.m. | #1
>>>>> "Simon" == Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes:


Simon> The change to macro_stringify is straightforward.  This allows removing
Simon> the manual memory management in fixup_definition.

Simon> @@ -882,25 +882,19 @@ macro_undef (struct macro_source_file *source, int line,
Simon>  static struct macro_definition *
Simon>  fixup_definition (const char *filename, int line, struct macro_definition *def)
Simon>  {
Simon> -  static char *saved_expansion;
[...]
Simon> +  gdb::unique_xmalloc_ptr<char> saved_expansion;
 
This loses the "static", but that's necessary because this function
returns "def", which has a pointer to the contents.

Tom
Shahab Vahedi via Gdb-patches July 1, 2020, 12:26 a.m. | #2
On 2020-06-30 4:55 p.m., Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes:

> 

> Simon> The change to macro_stringify is straightforward.  This allows removing

> Simon> the manual memory management in fixup_definition.

> 

> Simon> @@ -882,25 +882,19 @@ macro_undef (struct macro_source_file *source, int line,

> Simon>  static struct macro_definition *

> Simon>  fixup_definition (const char *filename, int line, struct macro_definition *def)

> Simon>  {

> Simon> -  static char *saved_expansion;

> [...]

> Simon> +  gdb::unique_xmalloc_ptr<char> saved_expansion;

>  

> This loses the "static", but that's necessary because this function

> returns "def", which has a pointer to the contents.


Oh damn, I fixed it last minute (it fails some tests) but probably forgot to git-add.

Here's the fixed version:


From e0ce95d17288009301534846fcb8c278f5d58fd7 Mon Sep 17 00:00:00 2001
From: Simon Marchi <simon.marchi@polymtl.ca>

Date: Sat, 27 Jun 2020 16:12:20 -0400
Subject: [PATCH] gdb: make macro_stringify return a gdb::unique_xmalloc_ptr<char>

The change to macro_stringify is straightforward.  This allows removing
the manual memory management in fixup_definition.

gdb/ChangeLog:

	* macroexp.h (macro_stringify): Return
	gdb::unique_xmalloc_ptr<char>.
	* macroexp.c (macro_stringify): Likewise.
	* macrotab.c (fixup_definition): Update.

Change-Id: Id7db8988bdbd569dd51c4f4655b00eb26db277cb
---

diff --git a/gdb/macroexp.c b/gdb/macroexp.c
index e1d185d..5f749ff 100644
--- a/gdb/macroexp.c
+++ b/gdb/macroexp.c
@@ -698,7 +698,7 @@

 /* See macroexp.h.  */

-char *
+gdb::unique_xmalloc_ptr<char>
 macro_stringify (const char *str)
 {
   int len = strlen (str);
@@ -707,7 +707,7 @@
   stringify (&buffer, str, len);
   buffer.appendc ('\0');

-  return buffer.release ().release ();
+  return buffer.release ();
 }

 
diff --git a/gdb/macroexp.h b/gdb/macroexp.h
index 511991c..2e29d02 100644
--- a/gdb/macroexp.h
+++ b/gdb/macroexp.h
@@ -78,9 +78,7 @@
 int macro_is_digit (int c);


-/* Stringify STR according to C rules and return an xmalloc'd pointer
-   to the result.  */
-
-char *macro_stringify (const char *str);
+/* Stringify STR according to C rules and return a null-terminated string.  */
+gdb::unique_xmalloc_ptr<char> macro_stringify (const char *str);

 #endif /* MACROEXP_H */
diff --git a/gdb/macrotab.c b/gdb/macrotab.c
index 63cd301..9ada436 100644
--- a/gdb/macrotab.c
+++ b/gdb/macrotab.c
@@ -882,25 +882,19 @@
 static struct macro_definition *
 fixup_definition (const char *filename, int line, struct macro_definition *def)
 {
-  static char *saved_expansion;
-
-  if (saved_expansion)
-    {
-      xfree (saved_expansion);
-      saved_expansion = NULL;
-    }
+  static gdb::unique_xmalloc_ptr<char> saved_expansion;

   if (def->kind == macro_object_like)
     {
       if (def->argc == macro_FILE)
 	{
 	  saved_expansion = macro_stringify (filename);
-	  def->replacement = saved_expansion;
+	  def->replacement = saved_expansion.get ();
 	}
       else if (def->argc == macro_LINE)
 	{
-	  saved_expansion = xstrprintf ("%d", line);
-	  def->replacement = saved_expansion;
+	  saved_expansion.reset (xstrprintf ("%d", line));
+	  def->replacement = saved_expansion.get ();
 	}
     }
Tom Tromey July 1, 2020, 3:36 p.m. | #3
>>>>> "Simon" == Simon Marchi <simon.marchi@polymtl.ca> writes:


Simon> Oh damn, I fixed it last minute (it fails some tests) but probably forgot to git-add.

Simon> Here's the fixed version:

Thanks, this one looks good.

Tom
Shahab Vahedi via Gdb-patches July 4, 2020, 2:28 a.m. | #4
On 2020-07-01 11:36 a.m., Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi <simon.marchi@polymtl.ca> writes:

> 

> Simon> Oh damn, I fixed it last minute (it fails some tests) but probably forgot to git-add.

> 

> Simon> Here's the fixed version:

> 

> Thanks, this one looks good.

> 

> Tom

> 


Thanks, I'll push the series.

Simon

Patch

diff --git a/gdb/macroexp.c b/gdb/macroexp.c
index e1d185d30c8e..5f749ffe8928 100644
--- a/gdb/macroexp.c
+++ b/gdb/macroexp.c
@@ -698,7 +698,7 @@  stringify (struct macro_buffer *dest, const char *arg, int len)
 
 /* See macroexp.h.  */
 
-char *
+gdb::unique_xmalloc_ptr<char>
 macro_stringify (const char *str)
 {
   int len = strlen (str);
@@ -707,7 +707,7 @@  macro_stringify (const char *str)
   stringify (&buffer, str, len);
   buffer.appendc ('\0');
 
-  return buffer.release ().release ();
+  return buffer.release ();
 }
 
 
diff --git a/gdb/macroexp.h b/gdb/macroexp.h
index 511991cacd20..2e29d02d34bd 100644
--- a/gdb/macroexp.h
+++ b/gdb/macroexp.h
@@ -78,9 +78,7 @@  int macro_is_identifier_nondigit (int c);
 int macro_is_digit (int c);
 
 
-/* Stringify STR according to C rules and return an xmalloc'd pointer
-   to the result.  */
-
-char *macro_stringify (const char *str);
+/* Stringify STR according to C rules and return a null-terminated string.  */
+gdb::unique_xmalloc_ptr<char> macro_stringify (const char *str);
 
 #endif /* MACROEXP_H */
diff --git a/gdb/macrotab.c b/gdb/macrotab.c
index 63cd30148ac2..0fefe8faaacf 100644
--- a/gdb/macrotab.c
+++ b/gdb/macrotab.c
@@ -882,25 +882,19 @@  macro_undef (struct macro_source_file *source, int line,
 static struct macro_definition *
 fixup_definition (const char *filename, int line, struct macro_definition *def)
 {
-  static char *saved_expansion;
-
-  if (saved_expansion)
-    {
-      xfree (saved_expansion);
-      saved_expansion = NULL;
-    }
+  gdb::unique_xmalloc_ptr<char> saved_expansion;
 
   if (def->kind == macro_object_like)
     {
       if (def->argc == macro_FILE)
 	{
 	  saved_expansion = macro_stringify (filename);
-	  def->replacement = saved_expansion;
+	  def->replacement = saved_expansion.get ();
 	}
       else if (def->argc == macro_LINE)
 	{
-	  saved_expansion = xstrprintf ("%d", line);
-	  def->replacement = saved_expansion;
+	  saved_expansion.reset (xstrprintf ("%d", line));
+	  def->replacement = saved_expansion.get ();
 	}
     }