Checks
Context |
Check |
Description |
linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 |
success
|
Testing passed
|
linaro-tcwg-bot/tcwg_gdb_build--master-arm |
success
|
Testing passed
|
linaro-tcwg-bot/tcwg_gdb_check--master-arm |
success
|
Testing passed
|
linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 |
success
|
Testing passed
|
Commit Message
The other day on irc, we were discussing the "m_line" hack in
tui-out.c, and I mentioned that it would be nice to replace this with
a new ui_out_flag.
Later, I looked at ui_out_flag and found:
ui_source_list = (1 << 0),
... and sure enough, this is tested already.
This patch removes tui-out.[ch] and changes the TUI to use an ordinary
cli-out object without this flag set.
As far as I can tell, this doesn't affect behavior at all -- the TUI
tests all pass, and interactively I tried switching stack frames,
"list", etc, and it all seems to work.
New in v2: fixed the problem pointed out by Keith, and added a test
case for that scenario.
---
gdb/Makefile.in | 2 -
gdb/source.c | 27 ++----
gdb/testsuite/gdb.tui/tui-missing-src.exp | 2 +
gdb/tui/tui-io.c | 3 +-
gdb/tui/tui-out.c | 111 ----------------------
gdb/tui/tui-out.h | 64 -------------
6 files changed, 12 insertions(+), 197 deletions(-)
delete mode 100644 gdb/tui/tui-out.c
delete mode 100644 gdb/tui/tui-out.h
Comments
Tom Tromey <tom@tromey.com> writes:
> The other day on irc, we were discussing the "m_line" hack in
> tui-out.c, and I mentioned that it would be nice to replace this with
> a new ui_out_flag.
>
> Later, I looked at ui_out_flag and found:
>
> ui_source_list = (1 << 0),
>
> ... and sure enough, this is tested already.
>
> This patch removes tui-out.[ch] and changes the TUI to use an ordinary
> cli-out object without this flag set.
>
> As far as I can tell, this doesn't affect behavior at all -- the TUI
> tests all pass, and interactively I tried switching stack frames,
> "list", etc, and it all seems to work.
>
> New in v2: fixed the problem pointed out by Keith, and added a test
> case for that scenario.
This LGTM. Keith might also want to look at this so I'll just add:
Reviewed-By: Andrew Burgess <aburgess@redhat.com>
Thanks,
Andrew
> ---
> gdb/Makefile.in | 2 -
> gdb/source.c | 27 ++----
> gdb/testsuite/gdb.tui/tui-missing-src.exp | 2 +
> gdb/tui/tui-io.c | 3 +-
> gdb/tui/tui-out.c | 111 ----------------------
> gdb/tui/tui-out.h | 64 -------------
> 6 files changed, 12 insertions(+), 197 deletions(-)
> delete mode 100644 gdb/tui/tui-out.c
> delete mode 100644 gdb/tui/tui-out.h
>
> diff --git a/gdb/Makefile.in b/gdb/Makefile.in
> index 0e0f19c40c9..340ff266a6e 100644
> --- a/gdb/Makefile.in
> +++ b/gdb/Makefile.in
> @@ -318,7 +318,6 @@ SUBDIR_TUI_SRCS = \
> tui/tui-io.c \
> tui/tui-layout.c \
> tui/tui-location.c \
> - tui/tui-out.c \
> tui/tui-regs.c \
> tui/tui-source.c \
> tui/tui-status.c \
> @@ -1605,7 +1604,6 @@ HFILES_NO_SRCDIR = \
> tui/tui-io.h \
> tui/tui-layout.h \
> tui/tui-location.h \
> - tui/tui-out.h \
> tui/tui-regs.h \
> tui/tui-source.h \
> tui/tui-status.h \
> diff --git a/gdb/source.c b/gdb/source.c
> index 2c465a1251f..bbeb4154258 100644
> --- a/gdb/source.c
> +++ b/gdb/source.c
> @@ -1341,30 +1341,21 @@ print_source_lines_base (struct symtab *s, int line, int stopline,
> styled_string (file_name_style.style (), filename),
> safe_strerror (errcode));
> }
> - else
> + else if (uiout->is_mi_like_p () || uiout->test_flags (ui_source_list))
> {
> + /* CLI expects only the "file" field. MI expects both
> + fields. ui_source_list is set only for CLI, not for
> + TUI. */
> +
> uiout->field_signed ("line", line);
> uiout->text ("\tin ");
>
> - /* CLI expects only the "file" field. TUI expects only the
> - "fullname" field (and TUI does break if "file" is printed).
> - MI expects both fields. ui_source_list is set only for CLI,
> - not for TUI. */
> - if (uiout->is_mi_like_p () || uiout->test_flags (ui_source_list))
> - uiout->field_string ("file", symtab_to_filename_for_display (s),
> - file_name_style.style ());
> - if (uiout->is_mi_like_p () || !uiout->test_flags (ui_source_list))
> + uiout->field_string ("file", symtab_to_filename_for_display (s),
> + file_name_style.style ());
> + if (uiout->is_mi_like_p ())
> {
> const char *s_fullname = symtab_to_fullname (s);
> - char *local_fullname;
> -
> - /* ui_out_field_string may free S_FULLNAME by calling
> - open_source_file for it again. See e.g.,
> - tui_field_string->tui_show_source. */
> - local_fullname = (char *) alloca (strlen (s_fullname) + 1);
> - strcpy (local_fullname, s_fullname);
> -
> - uiout->field_string ("fullname", local_fullname);
> + uiout->field_string ("fullname", s_fullname);
> }
>
> uiout->text ("\n");
> diff --git a/gdb/testsuite/gdb.tui/tui-missing-src.exp b/gdb/testsuite/gdb.tui/tui-missing-src.exp
> index 19da9389a46..1929a1e3568 100644
> --- a/gdb/testsuite/gdb.tui/tui-missing-src.exp
> +++ b/gdb/testsuite/gdb.tui/tui-missing-src.exp
> @@ -89,6 +89,8 @@ Term::check_box_contents "check source box is empty" \
> Term::command "set pagination off"
> Term::command "start"
> Term::command "next"
> +Term::check_contents "next does not show source line" \
> + "\\(gdb\\) next *\n\\(gdb\\)"
> Term::command "step"
> Term::check_contents "checking if inside f2 ()" "f2 \\(x=4\\)"
> Term::check_box_contents "f2.c must be displayed in source window" \
> diff --git a/gdb/tui/tui-io.c b/gdb/tui/tui-io.c
> index 94554fd447d..ba07e3d573b 100644
> --- a/gdb/tui/tui-io.c
> +++ b/gdb/tui/tui-io.c
> @@ -33,7 +33,6 @@
> #include "tui/tui-win.h"
> #include "tui/tui-wingeneral.h"
> #include "tui/tui-file.h"
> -#include "tui/tui-out.h"
> #include "ui-out.h"
> #include "cli-out.h"
> #include <fcntl.h>
> @@ -917,7 +916,7 @@ tui_initialize_io (void)
> tui_stdout = new pager_file (new tui_file (stdout, true));
> tui_stderr = new tui_file (stderr, false);
> tui_stdlog = new timestamped_file (tui_stderr);
> - tui_out = new tui_ui_out (tui_stdout);
> + tui_out = new cli_ui_out (tui_stdout, 0);
>
> /* Create the default UI. */
> tui_old_uiout = new cli_ui_out (gdb_stdout);
> diff --git a/gdb/tui/tui-out.c b/gdb/tui/tui-out.c
> deleted file mode 100644
> index 2ce7309b902..00000000000
> --- a/gdb/tui/tui-out.c
> +++ /dev/null
> @@ -1,111 +0,0 @@
> -/* Output generating routines for GDB CLI.
> -
> - Copyright (C) 1999-2024 Free Software Foundation, Inc.
> -
> - Contributed by Cygnus Solutions.
> - Written by Fernando Nasser for Cygnus.
> -
> - This file is part of GDB.
> -
> - This program is free software; you can redistribute it and/or modify
> - it under the terms of the GNU General Public License as published by
> - the Free Software Foundation; either version 3 of the License, or
> - (at your option) any later version.
> -
> - This program is distributed in the hope that it will be useful,
> - but WITHOUT ANY WARRANTY; without even the implied warranty of
> - MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> - GNU General Public License for more details.
> -
> - You should have received a copy of the GNU General Public License
> - along with this program. If not, see <http://www.gnu.org/licenses/>. */
> -
> -#include "defs.h"
> -#include "ui-out.h"
> -#include "tui-out.h"
> -#include "tui.h"
> -
> -/* Output an int field. */
> -
> -void
> -tui_ui_out::do_field_signed (int fldno, int width, ui_align alignment,
> - const char *fldname, LONGEST value)
> -{
> - if (suppress_output ())
> - return;
> -
> - /* Don't print line number, keep it for later. */
> - if (m_start_of_line == 0 && strcmp (fldname, "line") == 0)
> - {
> - m_start_of_line++;
> - m_line = value;
> - return;
> - }
> - m_start_of_line++;
> -
> - cli_ui_out::do_field_signed (fldno, width, alignment, fldname, value);
> -}
> -
> -/* Other cli_field_* end up here so alignment and field separators are
> - both handled by tui_field_string. */
> -
> -void
> -tui_ui_out::do_field_string (int fldno, int width, ui_align align,
> - const char *fldname, const char *string,
> - const ui_file_style &style)
> -{
> - if (suppress_output ())
> - return;
> -
> - m_start_of_line++;
> -
> - if (fldname && m_line > 0 && strcmp (fldname, "fullname") == 0)
> - return;
> -
> - cli_ui_out::do_field_string (fldno, width, align, fldname, string, style);
> -}
> -
> -void
> -tui_ui_out::do_field_fmt (int fldno, int width, ui_align align,
> - const char *fldname, const ui_file_style &style,
> - const char *format, va_list args)
> -{
> - if (suppress_output ())
> - return;
> -
> - m_start_of_line++;
> -
> - cli_ui_out::do_field_fmt (fldno, width, align, fldname, style, format, args);
> -}
> -
> -void
> -tui_ui_out::do_text (const char *string)
> -{
> - if (suppress_output ())
> - return;
> -
> - m_start_of_line++;
> - if (m_line > 0)
> - {
> - /* Printing a source line, so suppress regular output -- the
> - line was shown on the TUI's source window by tui_show_source
> - above instead. */
> - if (strchr (string, '\n') != 0)
> - {
> - /* We've reached the end of the line, so go back to letting
> - text output go to the console. */
> - m_line = 0;
> - m_start_of_line = 0;
> - }
> - return;
> - }
> - if (strchr (string, '\n'))
> - m_start_of_line = 0;
> -
> - cli_ui_out::do_text (string);
> -}
> -
> -tui_ui_out::tui_ui_out (ui_file *stream)
> - : cli_ui_out (stream, 0)
> -{
> -}
> diff --git a/gdb/tui/tui-out.h b/gdb/tui/tui-out.h
> deleted file mode 100644
> index e4ebb1e7408..00000000000
> --- a/gdb/tui/tui-out.h
> +++ /dev/null
> @@ -1,64 +0,0 @@
> -/* Copyright (C) 2016-2024 Free Software Foundation, Inc.
> -
> - This file is part of GDB.
> -
> - This program is free software; you can redistribute it and/or modify
> - it under the terms of the GNU General Public License as published by
> - the Free Software Foundation; either version 3 of the License, or
> - (at your option) any later version.
> -
> - This program is distributed in the hope that it will be useful,
> - but WITHOUT ANY WARRANTY; without even the implied warranty of
> - MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> - GNU General Public License for more details.
> -
> - You should have received a copy of the GNU General Public License
> - along with this program. If not, see <http://www.gnu.org/licenses/>. */
> -
> -#ifndef TUI_TUI_OUT_H
> -#define TUI_TUI_OUT_H
> -
> -#include "cli-out.h"
> -
> -/* A ui_out class for the TUI. This is just like the CLI's ui_out,
> - except that it overrides output methods to detect when a source
> - line is being printed and show the source in the TUI's source
> - window instead of printing the line in the console window. */
> -class tui_ui_out : public cli_ui_out
> -{
> -public:
> -
> - explicit tui_ui_out (ui_file *stream);
> -
> -protected:
> -
> - void do_field_signed (int fldno, int width, ui_align align, const char *fldname,
> - LONGEST value) override;
> - void do_field_string (int fldno, int width, ui_align align, const char *fldname,
> - const char *string, const ui_file_style &style) override;
> - void do_field_fmt (int fldno, int width, ui_align align, const char *fldname,
> - const ui_file_style &style,
> - const char *format, va_list args) override
> - ATTRIBUTE_PRINTF (7, 0);
> - void do_text (const char *string) override;
> -
> -private:
> -
> - /* These fields are used to make print_source_lines show the source
> - in the TUI's source window instead of in the console.
> - M_START_OF_LINE is incremented whenever something is output to
> - the ui_out. If an integer field named "line" is printed on the
> - ui_out, and nothing else has been printed yet (both
> - M_START_OF_LINE and M_LINE are still 0), we assume
> - print_source_lines is starting to print a source line, and thus
> - record the line number in M_LINE. Afterwards, when we see a
> - string field named "fullname" being output, we take the fullname
> - and the recorded line and show the source line in the TUI's
> - source window. tui_ui_out::do_text() suppresses text output
> - until it sees an endline being printed, at which point these
> - variables are reset back to 0. */
> - int m_line = 0;
> - int m_start_of_line = 0;
> -};
> -
> -#endif /* TUI_TUI_OUT_H */
> --
> 2.43.0
On 3/6/24 05:42, Andrew Burgess wrote:
>
> This LGTM. Keith might also want to look at this so I'll just add:
>
Yup, everything looks good from here. Thank you, Tom, for addressing
the problem.
Keith
@@ -318,7 +318,6 @@ SUBDIR_TUI_SRCS = \
tui/tui-io.c \
tui/tui-layout.c \
tui/tui-location.c \
- tui/tui-out.c \
tui/tui-regs.c \
tui/tui-source.c \
tui/tui-status.c \
@@ -1605,7 +1604,6 @@ HFILES_NO_SRCDIR = \
tui/tui-io.h \
tui/tui-layout.h \
tui/tui-location.h \
- tui/tui-out.h \
tui/tui-regs.h \
tui/tui-source.h \
tui/tui-status.h \
@@ -1341,30 +1341,21 @@ print_source_lines_base (struct symtab *s, int line, int stopline,
styled_string (file_name_style.style (), filename),
safe_strerror (errcode));
}
- else
+ else if (uiout->is_mi_like_p () || uiout->test_flags (ui_source_list))
{
+ /* CLI expects only the "file" field. MI expects both
+ fields. ui_source_list is set only for CLI, not for
+ TUI. */
+
uiout->field_signed ("line", line);
uiout->text ("\tin ");
- /* CLI expects only the "file" field. TUI expects only the
- "fullname" field (and TUI does break if "file" is printed).
- MI expects both fields. ui_source_list is set only for CLI,
- not for TUI. */
- if (uiout->is_mi_like_p () || uiout->test_flags (ui_source_list))
- uiout->field_string ("file", symtab_to_filename_for_display (s),
- file_name_style.style ());
- if (uiout->is_mi_like_p () || !uiout->test_flags (ui_source_list))
+ uiout->field_string ("file", symtab_to_filename_for_display (s),
+ file_name_style.style ());
+ if (uiout->is_mi_like_p ())
{
const char *s_fullname = symtab_to_fullname (s);
- char *local_fullname;
-
- /* ui_out_field_string may free S_FULLNAME by calling
- open_source_file for it again. See e.g.,
- tui_field_string->tui_show_source. */
- local_fullname = (char *) alloca (strlen (s_fullname) + 1);
- strcpy (local_fullname, s_fullname);
-
- uiout->field_string ("fullname", local_fullname);
+ uiout->field_string ("fullname", s_fullname);
}
uiout->text ("\n");
@@ -89,6 +89,8 @@ Term::check_box_contents "check source box is empty" \
Term::command "set pagination off"
Term::command "start"
Term::command "next"
+Term::check_contents "next does not show source line" \
+ "\\(gdb\\) next *\n\\(gdb\\)"
Term::command "step"
Term::check_contents "checking if inside f2 ()" "f2 \\(x=4\\)"
Term::check_box_contents "f2.c must be displayed in source window" \
@@ -33,7 +33,6 @@
#include "tui/tui-win.h"
#include "tui/tui-wingeneral.h"
#include "tui/tui-file.h"
-#include "tui/tui-out.h"
#include "ui-out.h"
#include "cli-out.h"
#include <fcntl.h>
@@ -917,7 +916,7 @@ tui_initialize_io (void)
tui_stdout = new pager_file (new tui_file (stdout, true));
tui_stderr = new tui_file (stderr, false);
tui_stdlog = new timestamped_file (tui_stderr);
- tui_out = new tui_ui_out (tui_stdout);
+ tui_out = new cli_ui_out (tui_stdout, 0);
/* Create the default UI. */
tui_old_uiout = new cli_ui_out (gdb_stdout);
deleted file mode 100644
@@ -1,111 +0,0 @@
-/* Output generating routines for GDB CLI.
-
- Copyright (C) 1999-2024 Free Software Foundation, Inc.
-
- Contributed by Cygnus Solutions.
- Written by Fernando Nasser for Cygnus.
-
- This file is part of GDB.
-
- This program is free software; you can redistribute it and/or modify
- it under the terms of the GNU General Public License as published by
- the Free Software Foundation; either version 3 of the License, or
- (at your option) any later version.
-
- This program is distributed in the hope that it will be useful,
- but WITHOUT ANY WARRANTY; without even the implied warranty of
- MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- GNU General Public License for more details.
-
- You should have received a copy of the GNU General Public License
- along with this program. If not, see <http://www.gnu.org/licenses/>. */
-
-#include "defs.h"
-#include "ui-out.h"
-#include "tui-out.h"
-#include "tui.h"
-
-/* Output an int field. */
-
-void
-tui_ui_out::do_field_signed (int fldno, int width, ui_align alignment,
- const char *fldname, LONGEST value)
-{
- if (suppress_output ())
- return;
-
- /* Don't print line number, keep it for later. */
- if (m_start_of_line == 0 && strcmp (fldname, "line") == 0)
- {
- m_start_of_line++;
- m_line = value;
- return;
- }
- m_start_of_line++;
-
- cli_ui_out::do_field_signed (fldno, width, alignment, fldname, value);
-}
-
-/* Other cli_field_* end up here so alignment and field separators are
- both handled by tui_field_string. */
-
-void
-tui_ui_out::do_field_string (int fldno, int width, ui_align align,
- const char *fldname, const char *string,
- const ui_file_style &style)
-{
- if (suppress_output ())
- return;
-
- m_start_of_line++;
-
- if (fldname && m_line > 0 && strcmp (fldname, "fullname") == 0)
- return;
-
- cli_ui_out::do_field_string (fldno, width, align, fldname, string, style);
-}
-
-void
-tui_ui_out::do_field_fmt (int fldno, int width, ui_align align,
- const char *fldname, const ui_file_style &style,
- const char *format, va_list args)
-{
- if (suppress_output ())
- return;
-
- m_start_of_line++;
-
- cli_ui_out::do_field_fmt (fldno, width, align, fldname, style, format, args);
-}
-
-void
-tui_ui_out::do_text (const char *string)
-{
- if (suppress_output ())
- return;
-
- m_start_of_line++;
- if (m_line > 0)
- {
- /* Printing a source line, so suppress regular output -- the
- line was shown on the TUI's source window by tui_show_source
- above instead. */
- if (strchr (string, '\n') != 0)
- {
- /* We've reached the end of the line, so go back to letting
- text output go to the console. */
- m_line = 0;
- m_start_of_line = 0;
- }
- return;
- }
- if (strchr (string, '\n'))
- m_start_of_line = 0;
-
- cli_ui_out::do_text (string);
-}
-
-tui_ui_out::tui_ui_out (ui_file *stream)
- : cli_ui_out (stream, 0)
-{
-}
deleted file mode 100644
@@ -1,64 +0,0 @@
-/* Copyright (C) 2016-2024 Free Software Foundation, Inc.
-
- This file is part of GDB.
-
- This program is free software; you can redistribute it and/or modify
- it under the terms of the GNU General Public License as published by
- the Free Software Foundation; either version 3 of the License, or
- (at your option) any later version.
-
- This program is distributed in the hope that it will be useful,
- but WITHOUT ANY WARRANTY; without even the implied warranty of
- MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- GNU General Public License for more details.
-
- You should have received a copy of the GNU General Public License
- along with this program. If not, see <http://www.gnu.org/licenses/>. */
-
-#ifndef TUI_TUI_OUT_H
-#define TUI_TUI_OUT_H
-
-#include "cli-out.h"
-
-/* A ui_out class for the TUI. This is just like the CLI's ui_out,
- except that it overrides output methods to detect when a source
- line is being printed and show the source in the TUI's source
- window instead of printing the line in the console window. */
-class tui_ui_out : public cli_ui_out
-{
-public:
-
- explicit tui_ui_out (ui_file *stream);
-
-protected:
-
- void do_field_signed (int fldno, int width, ui_align align, const char *fldname,
- LONGEST value) override;
- void do_field_string (int fldno, int width, ui_align align, const char *fldname,
- const char *string, const ui_file_style &style) override;
- void do_field_fmt (int fldno, int width, ui_align align, const char *fldname,
- const ui_file_style &style,
- const char *format, va_list args) override
- ATTRIBUTE_PRINTF (7, 0);
- void do_text (const char *string) override;
-
-private:
-
- /* These fields are used to make print_source_lines show the source
- in the TUI's source window instead of in the console.
- M_START_OF_LINE is incremented whenever something is output to
- the ui_out. If an integer field named "line" is printed on the
- ui_out, and nothing else has been printed yet (both
- M_START_OF_LINE and M_LINE are still 0), we assume
- print_source_lines is starting to print a source line, and thus
- record the line number in M_LINE. Afterwards, when we see a
- string field named "fullname" being output, we take the fullname
- and the recorded line and show the source line in the TUI's
- source window. tui_ui_out::do_text() suppresses text output
- until it sees an endline being printed, at which point these
- variables are reset back to 0. */
- int m_line = 0;
- int m_start_of_line = 0;
-};
-
-#endif /* TUI_TUI_OUT_H */