Message ID | 20180430051207.19979-5-tom@tromey.com |
---|---|
State | New |
Headers | show |
Series |
|
Related | show |
On 04/30/2018 06:12 AM, Tom Tromey wrote: > This removes the interp_name function. It is only used a few spots -- > one of which was only calling it on "this". It's simpler to remove > it; and should class interp become opaque in the future, it will be > just as easy to update the two remaining spots to use an accessor. Yeah, IIRC, it used to be opaque until I C++ified it. Not seeing a need to go back. > > ChangeLog > 2018-04-29 Tom Tromey <tom@tromey.com> > > * interps.c (interp_name): Remove. > * mi/mi-interp.c (mi_interp::init): Update. > * interps.h (interp_name): Remove. > (~scoped_restore_interp): Update. > * tui/tui.c (tui_enable): Update. > diff --git a/gdb/tui/tui.c b/gdb/tui/tui.c > index 7943a61676..9e2520b4fc 100644 > --- a/gdb/tui/tui.c > +++ b/gdb/tui/tui.c > @@ -415,7 +415,7 @@ tui_enable (void) > > /* If the top level interpreter is not the console/tui (e.g., > MI), enabling curses will certainly lose. */ > - interp = interp_name (top_level_interpreter ()); > + interp = top_level_interpreter ()->name; > if (strcmp (interp, INTERP_TUI) != 0) > error (_("Cannot enable the TUI when the interpreter is '%s'"), interp); Not sure. The name is supposedly read-only, which would suggest renaming the field to m_name and adding a name() getter. WDYT? Thanks, Pedro Alves
>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:
Pedro> Not sure. The name is supposedly read-only, which would
Pedro> suggest renaming the field to m_name and adding a name()
Pedro> getter. WDYT?
How about the appended as a follow-up patch to this series?
Tom
commit d525a99be1b02dda6c69007e31dd06f276378aea
Author: Tom Tromey <tom@tromey.com>
Date: Fri May 25 12:39:51 2018 -0600
Add "name" method to class interp
In a review Pedro pointed out that interp::name is intended to be
read-only, and so an accessor would be a better fit. This patch
renames the field and adds a "name" method that is used instead.
ChangeLog
2018-05-25 Tom Tromey <tom@tromey.com>
* tui/tui.c (tui_enable): Update.
* mi/mi-interp.c (mi_interp::init): Update.
* interps.h (class interp) <name>: New method.
<m_name>: Rename from name.
(~scoped_restore_interp): Update.
* interps.c (interp::interp): Update.
(interp_add, interp_set, interp_lookup_existing)
(current_interp_named_p): Update.
diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 0bf350c255..a01f60c3b0 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,14 @@
+2018-05-25 Tom Tromey <tom@tromey.com>
+
+ * tui/tui.c (tui_enable): Update.
+ * mi/mi-interp.c (mi_interp::init): Update.
+ * interps.h (class interp) <name>: New method.
+ <m_name>: Rename from name.
+ (~scoped_restore_interp): Update.
+ * interps.c (interp::interp): Update.
+ (interp_add, interp_set, interp_lookup_existing)
+ (current_interp_named_p): Update.
+
2018-05-25 Tom Tromey <tom@tromey.com>
* interps.c (interp_name): Remove.
diff --git a/gdb/interps.c b/gdb/interps.c
index 8ec9744fdf..789ae86946 100644
--- a/gdb/interps.c
+++ b/gdb/interps.c
@@ -78,8 +78,8 @@ static struct interp *interp_lookup_existing (struct ui *ui,
const char *name);
interp::interp (const char *name)
+ : m_name (xstrdup (name))
{
- this->name = xstrdup (name);
this->inited = false;
}
@@ -129,7 +129,7 @@ interp_add (struct ui *ui, struct interp *interp)
{
struct ui_interp_info *ui_interp = get_interp_info (ui);
- gdb_assert (interp_lookup_existing (ui, interp->name) == NULL);
+ gdb_assert (interp_lookup_existing (ui, interp->name ()) == NULL);
interp->next = ui_interp->interp_list;
ui_interp->interp_list = interp;
@@ -170,11 +170,11 @@ interp_set (struct interp *interp, bool top_level)
/* We use interpreter_p for the "set interpreter" variable, so we need
to make sure we have a malloc'ed copy for the set command to free. */
if (interpreter_p != NULL
- && strcmp (interp->name, interpreter_p) != 0)
+ && strcmp (interp->name (), interpreter_p) != 0)
{
xfree (interpreter_p);
- interpreter_p = xstrdup (interp->name);
+ interpreter_p = xstrdup (interp->name ());
}
/* Run the init proc. */
@@ -206,7 +206,7 @@ interp_lookup_existing (struct ui *ui, const char *name)
interp != NULL;
interp = interp->next)
{
- if (strcmp (interp->name, name) == 0)
+ if (strcmp (interp->name (), name) == 0)
return interp;
}
@@ -282,7 +282,7 @@ current_interp_named_p (const char *interp_name)
struct interp *interp = ui_interp->current_interpreter;
if (interp != NULL)
- return (strcmp (interp->name, interp_name) == 0);
+ return (strcmp (interp->name (), interp_name) == 0);
return 0;
}
diff --git a/gdb/interps.h b/gdb/interps.h
index a689be5625..74c9a80918 100644
--- a/gdb/interps.h
+++ b/gdb/interps.h
@@ -74,8 +74,13 @@ public:
virtual bool supports_command_editing ()
{ return false; }
+ const char *name () const
+ {
+ return m_name;
+ }
+
/* This is the name in "-i=" and "set interpreter". */
- const char *name;
+ const char *m_name;
/* Interpreters are stored in a linked list, this is the next
one... */
@@ -111,7 +116,7 @@ public:
~scoped_restore_interp ()
{
- set_interp (m_interp->name);
+ set_interp (m_interp->name ());
}
scoped_restore_interp (const scoped_restore_interp &) = delete;
diff --git a/gdb/mi/mi-interp.c b/gdb/mi/mi-interp.c
index e52d797973..ebc899f631 100644
--- a/gdb/mi/mi-interp.c
+++ b/gdb/mi/mi-interp.c
@@ -130,13 +130,13 @@ mi_interp::init (bool top_level)
/* INTERP_MI selects the most recent released version. "mi2" was
released as part of GDB 6.0. */
- if (strcmp (name, INTERP_MI) == 0)
+ if (strcmp (name (), INTERP_MI) == 0)
mi_version = 2;
- else if (strcmp (name, INTERP_MI1) == 0)
+ else if (strcmp (name (), INTERP_MI1) == 0)
mi_version = 1;
- else if (strcmp (name, INTERP_MI2) == 0)
+ else if (strcmp (name (), INTERP_MI2) == 0)
mi_version = 2;
- else if (strcmp (name, INTERP_MI3) == 0)
+ else if (strcmp (name (), INTERP_MI3) == 0)
mi_version = 3;
else
gdb_assert_not_reached ("unhandled MI version");
diff --git a/gdb/tui/tui.c b/gdb/tui/tui.c
index 9e2520b4fc..75a9ced619 100644
--- a/gdb/tui/tui.c
+++ b/gdb/tui/tui.c
@@ -415,7 +415,7 @@ tui_enable (void)
/* If the top level interpreter is not the console/tui (e.g.,
MI), enabling curses will certainly lose. */
- interp = top_level_interpreter ()->name;
+ interp = top_level_interpreter ()->name ();
if (strcmp (interp, INTERP_TUI) != 0)
error (_("Cannot enable the TUI when the interpreter is '%s'"), interp);
On 05/25/2018 07:41 PM, Tom Tromey wrote: >>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes: > > Pedro> Not sure. The name is supposedly read-only, which would > Pedro> suggest renaming the field to m_name and adding a name() > Pedro> getter. WDYT? > > How about the appended as a follow-up patch to this series? Thanks, looks good. Pedro Alves
diff --git a/gdb/interps.c b/gdb/interps.c index 5884625856..8ec9744fdf 100644 --- a/gdb/interps.c +++ b/gdb/interps.c @@ -274,14 +274,6 @@ scoped_restore_interp::set_interp (const char *name) return old_interp; } -/* Returns the interpreter's name. */ - -const char * -interp_name (struct interp *interp) -{ - return interp->name; -} - /* Returns true if the current interp is the passed in name. */ int current_interp_named_p (const char *interp_name) diff --git a/gdb/interps.h b/gdb/interps.h index 694149a6ca..a689be5625 100644 --- a/gdb/interps.h +++ b/gdb/interps.h @@ -98,8 +98,6 @@ extern struct interp *interp_lookup (struct ui *ui, const char *name); interpreter fails to initialize. */ extern void set_top_level_interpreter (const char *name); -extern const char *interp_name (struct interp *interp); - /* Temporarily set the current interpreter, and reset it on destruction. */ class scoped_restore_interp @@ -113,7 +111,7 @@ public: ~scoped_restore_interp () { - set_interp (interp_name (m_interp)); + set_interp (m_interp->name); } scoped_restore_interp (const scoped_restore_interp &) = delete; diff --git a/gdb/mi/mi-interp.c b/gdb/mi/mi-interp.c index 99ce1eb5d6..e52d797973 100644 --- a/gdb/mi/mi-interp.c +++ b/gdb/mi/mi-interp.c @@ -113,7 +113,6 @@ void mi_interp::init (bool top_level) { mi_interp *mi = this; - const char *name; int mi_version; /* Store the current output channel, so that we can create a console @@ -129,7 +128,6 @@ mi_interp::init (bool top_level) mi->targ = new mi_console_file (mi->raw_stdout, "@", '"'); mi->event_channel = new mi_console_file (mi->raw_stdout, "=", 0); - name = interp_name (this); /* INTERP_MI selects the most recent released version. "mi2" was released as part of GDB 6.0. */ if (strcmp (name, INTERP_MI) == 0) diff --git a/gdb/tui/tui.c b/gdb/tui/tui.c index 7943a61676..9e2520b4fc 100644 --- a/gdb/tui/tui.c +++ b/gdb/tui/tui.c @@ -415,7 +415,7 @@ tui_enable (void) /* If the top level interpreter is not the console/tui (e.g., MI), enabling curses will certainly lose. */ - interp = interp_name (top_level_interpreter ()); + interp = top_level_interpreter ()->name; if (strcmp (interp, INTERP_TUI) != 0) error (_("Cannot enable the TUI when the interpreter is '%s'"), interp);