Patchwork Factor out mi_ui_out instantiation logic

login
register
mail settings
Submitter Simon Marchi
Date March 13, 2019, 3:02 a.m.
Message ID <20190313030218.21452-1-simon.marchi@polymtl.ca>
Download mbox | patch
Permalink /patch/31834/
State New
Headers show

Comments

Simon Marchi - March 13, 2019, 3:02 a.m.
When re-reviewing this [1] I noticed that there were two spots encoding
the logic of instantiating an mi_ui_out object based on the interpreter
name ("mi", "mi1", "mi2" or "mi3"):

 - mi_interp::init
 - mi_load_progress

Both encode the logic to choose what the default version is when the
interpreter name is "mi".  I had forgotten the one in mi_load_progress.

Therefore, I propose extracting that logic to a single function.  I
started to add a new overload of mi_out_new, then realized the current
mi_out_new wasn't very useful, being just a thing wrapper around "new
mi_ui_out".  So I ended up with just an mi_out_new function taking the
interp name as parameter.

I ran the gdb.mi tests, and verified manually the behavior (including
the load command).

[1] https://sourceware.org/ml/gdb-patches/2019-01/msg00427.html

gdb/ChangeLog:

	* mi/mi-out.h (mi_out_new): Change parameter to const char *.
	* mi/mi-out.c (mi_out_new): Change parameter to const char *,
	instantiate mi_ui_out based on interpreter name.
	* mi/mi-interp.c (mi_interp::init): Use the new mi_out_new.
	* mi/mi-main.c (mi_load_progress): Likewise.
---
 gdb/mi/mi-interp.c | 18 ++----------------
 gdb/mi/mi-main.c   | 12 ++----------
 gdb/mi/mi-out.c    | 21 +++++++++++++++++----
 gdb/mi/mi-out.h    |  7 ++++++-
 4 files changed, 27 insertions(+), 31 deletions(-)
Pedro Alves - March 13, 2019, 3:30 p.m.
On 03/13/2019 03:02 AM, Simon Marchi wrote:

> gdb/ChangeLog:
> 
> 	* mi/mi-out.h (mi_out_new): Change parameter to const char *.
> 	* mi/mi-out.c (mi_out_new): Change parameter to const char *,
> 	instantiate mi_ui_out based on interpreter name.
> 	* mi/mi-interp.c (mi_interp::init): Use the new mi_out_new.

LGTM.

Thanks,
Pedro Alves
Simon Marchi - March 13, 2019, 5:33 p.m.
On 2019-03-13 11:30 a.m., Pedro Alves wrote:
> On 03/13/2019 03:02 AM, Simon Marchi wrote:
> 
>> gdb/ChangeLog:
>>
>> 	* mi/mi-out.h (mi_out_new): Change parameter to const char *.
>> 	* mi/mi-out.c (mi_out_new): Change parameter to const char *,
>> 	instantiate mi_ui_out based on interpreter name.
>> 	* mi/mi-interp.c (mi_interp::init): Use the new mi_out_new.
> 
> LGTM.
> 
> Thanks,
> Pedro Alves
> 

Thanks, I pushed it.

Simon

Patch

diff --git a/gdb/mi/mi-interp.c b/gdb/mi/mi-interp.c
index 3e9f36897a86..f17e09ff04f5 100644
--- a/gdb/mi/mi-interp.c
+++ b/gdb/mi/mi-interp.c
@@ -114,7 +114,6 @@  void
 mi_interp::init (bool top_level)
 {
   mi_interp *mi = this;
-  int mi_version;
 
   /* Store the current output channel, so that we can create a console
      channel that encapsulates and prefixes all gdb_output-type bits
@@ -128,21 +127,8 @@  mi_interp::init (bool top_level)
   mi->log = mi->err;
   mi->targ = new mi_console_file (mi->raw_stdout, "@", '"');
   mi->event_channel = new mi_console_file (mi->raw_stdout, "=", 0);
-
-  /* INTERP_MI selects the most recent released version.  "mi2" was
-     released as part of GDB 6.0.  */
-  if (strcmp (name (), INTERP_MI) == 0)
-    mi_version = 2;
-  else if (strcmp (name (), INTERP_MI1) == 0)
-    mi_version = 1;
-  else if (strcmp (name (), INTERP_MI2) == 0)
-    mi_version = 2;
-  else if (strcmp (name (), INTERP_MI3) == 0)
-    mi_version = 3;
-  else
-    gdb_assert_not_reached ("unhandled MI version");
-
-  mi->mi_uiout = mi_out_new (mi_version);
+  mi->mi_uiout = mi_out_new (name ());
+  gdb_assert (mi->mi_uiout != nullptr);
   mi->cli_uiout = cli_out_new (mi->out);
 
   if (top_level)
diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
index 46bc928d9fe9..f4e5e48bc221 100644
--- a/gdb/mi/mi-main.c
+++ b/gdb/mi/mi-main.c
@@ -2173,16 +2173,8 @@  mi_load_progress (const char *section_name,
      which means uiout may not be correct.  Fix it for the duration
      of this function.  */
 
-  std::unique_ptr<ui_out> uiout;
-
-  if (current_interp_named_p (INTERP_MI)
-      || current_interp_named_p (INTERP_MI2))
-    uiout.reset (mi_out_new (2));
-  else if (current_interp_named_p (INTERP_MI1))
-    uiout.reset (mi_out_new (1));
-  else if (current_interp_named_p (INTERP_MI3))
-    uiout.reset (mi_out_new (3));
-  else
+  std::unique_ptr<ui_out> uiout (mi_out_new (current_interpreter ()->name ()));
+  if (uiout == nullptr)
     return;
 
   scoped_restore save_uiout
diff --git a/gdb/mi/mi-out.c b/gdb/mi/mi-out.c
index 11991dca6639..4f8c9bff8e58 100644
--- a/gdb/mi/mi-out.c
+++ b/gdb/mi/mi-out.c
@@ -20,10 +20,14 @@ 
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
 #include "defs.h"
-#include "ui-out.h"
 #include "mi-out.h"
+
 #include <vector>
 
+#include "interps.h"
+#include "ui-out.h"
+#include "utils.h"
+
 /* Mark beginning of a table.  */
 
 void
@@ -288,12 +292,21 @@  mi_ui_out::~mi_ui_out ()
 {
 }
 
-/* Initialize private members at startup.  */
+/* See mi/mi-out.h.  */
 
 mi_ui_out *
-mi_out_new (int mi_version)
+mi_out_new (const char *mi_version)
 {
-  return new mi_ui_out (mi_version);
+  if (streq (mi_version, INTERP_MI3))
+    return new mi_ui_out (3);
+
+  if (streq (mi_version, INTERP_MI2) || streq (mi_version, INTERP_MI))
+    return new mi_ui_out (2);
+
+  if (streq (mi_version, INTERP_MI1))
+    return new mi_ui_out (1);
+
+  return nullptr;
 }
 
 /* Helper function to return the given UIOUT as an mi_ui_out.  It is an error
diff --git a/gdb/mi/mi-out.h b/gdb/mi/mi-out.h
index 0c74eaf80fda..82f77592da84 100644
--- a/gdb/mi/mi-out.h
+++ b/gdb/mi/mi-out.h
@@ -90,7 +90,12 @@  private:
   std::vector<ui_file *> m_streams;
 };
 
-mi_ui_out *mi_out_new (int mi_version);
+/* Create an MI ui-out object with MI version MI_VERSION, which should be equal
+   to one of the INTERP_MI* constants (see interps.h).
+
+   Return nullptr if an invalid version is provided.  */
+mi_ui_out *mi_out_new (const char *mi_version);
+
 int mi_version (ui_out *uiout);
 void mi_out_put (ui_out *uiout, struct ui_file *stream);
 void mi_out_rewind (ui_out *uiout);