Factor out mi_ui_out instantiation logic
Commit Message
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(-)
Comments
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
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
@@ -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)
@@ -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
@@ -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
@@ -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);