Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Simon Marchi <simon.marchi@polymtl.ca>
To: gdb-patches@sourceware.org
Cc: Simon Marchi <simon.marchi@polymtl.ca>
Subject: [PATCH] Factor out mi_ui_out instantiation logic
Date: Wed, 13 Mar 2019 03:02:00 -0000	[thread overview]
Message-ID: <20190313030218.21452-1-simon.marchi@polymtl.ca> (raw)

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(-)

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);
-- 
2.21.0


             reply	other threads:[~2019-03-13  3:02 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-03-13  3:02 Simon Marchi [this message]
2019-03-13 15:31 ` Pedro Alves
2019-03-13 17:34   ` Simon Marchi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190313030218.21452-1-simon.marchi@polymtl.ca \
    --to=simon.marchi@polymtl.ca \
    --cc=gdb-patches@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox