diff mbox series

[2/3] gdb: make interp::m_name an `const char *`

Message ID 20230302203224.118345-3-simon.marchi@efficios.com
State New
Headers
Series Some minor interps tweaks |

Commit Message

Simon Marchi March 2, 2023, 8:32 p.m. UTC 
  From: Simon Marchi <simon.marchi@polymtl.ca>

I realized that the memory for interp names does not need to be
allocated.  The name used to register interp factory functions is always
a literal string, so has static storage duration.  If we change
interp_lookup to pass that name instead of the string that it receives
as a parameter (which does not always have static storage duration),
then interps can simply store pointers to the name.

So, change interp_lookup to pass `factory.name` rather than `name`.
Change interp::m_name to be a `const char *` rather than an std::string.

Change-Id: I0474d1f7b3512e7d172ccd73018aea927def3188
---
 gdb/interps.c |  8 +++-----
 gdb/interps.h | 12 ++++++------
 2 files changed, 9 insertions(+), 11 deletions(-)

Comments

Tom Tromey March 7, 2023, 7:29 p.m. UTC | #1 
>>>>> "Simon" == Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes:

Simon> I realized that the memory for interp names does not need to be
Simon> allocated.  The name used to register interp factory functions is always
Simon> a literal string, so has static storage duration.  If we change
Simon> interp_lookup to pass that name instead of the string that it receives
Simon> as a parameter (which does not always have static storage duration),
Simon> then interps can simply store pointers to the name.

I wish we had a way to detect/enforce this.  That way lies rust I suppose.

Simon> So, change interp_lookup to pass `factory.name` rather than `name`.
Simon> Change interp::m_name to be a `const char *` rather than an std::string.

Reviewed-By: Tom Tromey <tom@tromey.com>

Tom
diff mbox series

Patch

diff --git a/gdb/interps.c b/gdb/interps.c
index 9517e57540d3..5b964aebbbd6 100644
--- a/gdb/interps.c
+++ b/gdb/interps.c
@@ -79,13 +79,11 @@  static struct interp *interp_lookup_existing (struct ui *ui,
 					      const char *name);
 
 interp::interp (const char *name)
-  : m_name (make_unique_xstrdup (name))
+  : m_name (name)
 {
 }
 
-interp::~interp ()
-{
-}
+interp::~interp () = default;
 
 /* An interpreter factory.  Maps an interpreter name to the factory
    function that instantiates an interpreter by that name.  */
@@ -232,7 +230,7 @@  interp_lookup (struct ui *ui, const char *name)
   for (const interp_factory &factory : interpreter_factories)
     if (strcmp (factory.name, name) == 0)
       {
-	interp = factory.func (name);
+	interp = factory.func (factory.name);
 	interp_add (ui, interp);
 	return interp;
       }
diff --git a/gdb/interps.h b/gdb/interps.h
index 01bec47550dd..62f37951ddea 100644
--- a/gdb/interps.h
+++ b/gdb/interps.h
@@ -32,7 +32,9 @@  typedef struct interp *(*interp_factory_func) (const char *name);
 /* Each interpreter kind (CLI, MI, etc.) registers itself with a call
    to this function, passing along its name, and a pointer to a
    function that creates a new instance of an interpreter with that
-   name.  */
+   name.
+
+   The memory for NAME must have static storage duration.  */
 extern void interp_factory_register (const char *name,
 				     interp_factory_func func);
 
@@ -76,13 +78,11 @@  class interp
   { return false; }
 
   const char *name () const
-  {
-    return m_name.get ();
-  }
+  { return m_name; }
 
 private:
-  /* This is the name in "-i=" and "set interpreter".  */
-  gdb::unique_xmalloc_ptr<char> m_name;
+  /* The memory for this is static, it comes from literal strings (e.g. "cli").  */
+  const char *m_name;
 
 public:
   /* Interpreters are stored in a linked list, this is the next