Change return type of ui_out redirect to void
Commit Message
All implementations of redirect/do_redirect in the ui_out subsystem
always return 0 (success). We can therefore clean it up and make them
return void.
gdb/ChangeLog:
* cli-out.c (cli_ui_out::do_redirect): Change return type to
void.
* cli-out.h (cli_ui_out::do_redirect): Likewise.
* mi/mi-out.c (mi_ui_out::do_redirect): Likewise.
* mi/mi-out.h (mi_ui_out::do_redirect): Likewise.
* ui-out.c (ui_out::redirect): Likewise.
* ui-out.h (ui_out::redirect, ui_out::do_redirect): Likewise.
* cli/cli-logging.c (set_logging_redirect): Update call site of
ui_out::redirect.
(handle_redirections): Likewise.
* scm-ports.c (ioscm_with_output_to_port_worker): Likewise.
* top.c (execute_command_to_string): Likewise.
* utils.c (do_ui_out_redirect_pop): Likewise.
---
gdb/cli-out.c | 4 +---
gdb/cli-out.h | 2 +-
gdb/cli/cli-logging.c | 15 +++++----------
gdb/guile/scm-ports.c | 6 ++----
gdb/mi/mi-out.c | 4 +---
gdb/mi/mi-out.h | 2 +-
gdb/top.c | 6 ++----
gdb/ui-out.c | 4 ++--
gdb/ui-out.h | 4 ++--
gdb/utils.c | 3 +--
10 files changed, 18 insertions(+), 32 deletions(-)
Comments
On 12/22/2016 04:07 PM, Simon Marchi wrote:
> All implementations of redirect/do_redirect in the ui_out subsystem
> always return 0 (success). We can therefore clean it up and make them
> return void.
>
> gdb/ChangeLog:
>
> * cli-out.c (cli_ui_out::do_redirect): Change return type to
> void.
> * cli-out.h (cli_ui_out::do_redirect): Likewise.
> * mi/mi-out.c (mi_ui_out::do_redirect): Likewise.
> * mi/mi-out.h (mi_ui_out::do_redirect): Likewise.
> * ui-out.c (ui_out::redirect): Likewise.
> * ui-out.h (ui_out::redirect, ui_out::do_redirect): Likewise.
> * cli/cli-logging.c (set_logging_redirect): Update call site of
> ui_out::redirect.
> (handle_redirections): Likewise.
> * scm-ports.c (ioscm_with_output_to_port_worker): Likewise.
> * top.c (execute_command_to_string): Likewise.
> * utils.c (do_ui_out_redirect_pop): Likewise.
> ---
> gdb/cli-out.c | 4 +---
> gdb/cli-out.h | 2 +-
> gdb/cli/cli-logging.c | 15 +++++----------
> gdb/guile/scm-ports.c | 6 ++----
> gdb/mi/mi-out.c | 4 +---
> gdb/mi/mi-out.h | 2 +-
> gdb/top.c | 6 ++----
> gdb/ui-out.c | 4 ++--
> gdb/ui-out.h | 4 ++--
> gdb/utils.c | 3 +--
> 10 files changed, 18 insertions(+), 32 deletions(-)
>
> diff --git a/gdb/cli-out.c b/gdb/cli-out.c
> index af983a1a1f..d84ae37cac 100644
> --- a/gdb/cli-out.c
> +++ b/gdb/cli-out.c
> @@ -227,15 +227,13 @@ cli_ui_out::do_flush ()
> and make it therefore active. OUTSTREAM as NULL will pop the last pushed
> output stream; it is an internal error if it does not exist. */
>
> -int
> +void
> cli_ui_out::do_redirect (ui_file *outstream)
> {
> if (outstream != NULL)
> m_streams.push_back (outstream);
> else
> m_streams.pop_back ();
> -
> - return 0;
> }
>
> /* local functions */
> diff --git a/gdb/cli-out.h b/gdb/cli-out.h
> index ef996c41dc..e86fda2760 100644
> --- a/gdb/cli-out.h
> +++ b/gdb/cli-out.h
> @@ -60,7 +60,7 @@ protected:
> ATTRIBUTE_PRINTF (2,0);
> virtual void do_wrap_hint (const char *identstring) override;
> virtual void do_flush () override;
> - virtual int do_redirect (struct ui_file * outstream) override;
> + virtual void do_redirect (struct ui_file * outstream) override;
>
> bool suppress_output ()
> { return m_suppress_output; }
> diff --git a/gdb/cli/cli-logging.c b/gdb/cli/cli-logging.c
> index 46c23064fc..02e2705310 100644
> --- a/gdb/cli/cli-logging.c
> +++ b/gdb/cli/cli-logging.c
> @@ -130,13 +130,11 @@ set_logging_redirect (char *args, int from_tty, struct cmd_list_element *c)
>
> /* There is a former output pushed on the ui_out_redirect stack. We
> want to replace it by OUTPUT so we must pop the former value
> - first. We should either do both the pop and push or to do
> - neither of it. At least do not try to push OUTPUT if the pop
> - already failed. */
> + first. Ideally, we should either do both the pop and push or to do
> + neither of it. */
This comment reads funny. Maybe we should fix it while at it.
"... we should either do both the pop and push or do neither of them. "?
>
> - if (uiout->redirect (NULL) < 0
> - || uiout->redirect (output) < 0)
> - warning (_("Current output protocol does not support redirection"));
> + uiout->redirect (NULL);
> + uiout->redirect (output);
>
> do_cleanups (cleanups);
> }
> @@ -246,10 +244,7 @@ handle_redirections (int from_tty)
>
> /* Don't do the redirect for MI, it confuses MI's ui-out scheme. */
> if (!current_uiout->is_mi_like_p ())
> - {
> - if (current_uiout->redirect (output) < 0)
> - warning (_("Current output protocol does not support redirection"));
> - }
> + current_uiout->redirect (output);
> }
>
> static void
> diff --git a/gdb/guile/scm-ports.c b/gdb/guile/scm-ports.c
> index 68f2f8d0b9..6603698788 100644
> --- a/gdb/guile/scm-ports.c
> +++ b/gdb/guile/scm-ports.c
> @@ -531,10 +531,8 @@ ioscm_with_output_to_port_worker (SCM port, SCM thunk, enum oport oport,
> gdb_stderr = port_file;
> else
> {
> - if (current_uiout->redirect (port_file) < 0)
> - warning (_("Current output protocol does not support redirection"));
> - else
> - make_cleanup_ui_out_redirect_pop (current_uiout);
> + current_uiout->redirect (port_file);
> + make_cleanup_ui_out_redirect_pop (current_uiout);
>
> gdb_stdout = port_file;
> }
> diff --git a/gdb/mi/mi-out.c b/gdb/mi/mi-out.c
> index 3761a5c0e5..e9e657ba0f 100644
> --- a/gdb/mi/mi-out.c
> +++ b/gdb/mi/mi-out.c
> @@ -170,15 +170,13 @@ mi_ui_out::do_flush ()
> gdb_flush (m_streams.back ());
> }
>
> -int
> +void
> mi_ui_out::do_redirect (ui_file *outstream)
> {
> if (outstream != NULL)
> m_streams.push_back (outstream);
> else
> m_streams.pop_back ();
> -
> - return 0;
> }
>
> void
> diff --git a/gdb/mi/mi-out.h b/gdb/mi/mi-out.h
> index 933f1d0d98..21e6880a41 100644
> --- a/gdb/mi/mi-out.h
> +++ b/gdb/mi/mi-out.h
> @@ -67,7 +67,7 @@ protected:
> ATTRIBUTE_PRINTF (2,0);
> virtual void do_wrap_hint (const char *identstring) override;
> virtual void do_flush () override;
> - virtual int do_redirect (struct ui_file * outstream) override;
> + virtual void do_redirect (struct ui_file * outstream) override;
>
> virtual bool do_is_mi_like_p () override
> { return true; }
> diff --git a/gdb/top.c b/gdb/top.c
> index 077fb2afec..49034ee454 100644
> --- a/gdb/top.c
> +++ b/gdb/top.c
> @@ -706,10 +706,8 @@ execute_command_to_string (char *p, int from_tty)
>
> make_cleanup_ui_file_delete (str_file);
>
> - if (current_uiout->redirect (str_file) < 0)
> - warning (_("Current output protocol does not support redirection"));
> - else
> - make_cleanup_ui_out_redirect_pop (current_uiout);
> + current_uiout->redirect (str_file);
> + make_cleanup_ui_out_redirect_pop (current_uiout);
>
> scoped_restore save_stdout
> = make_scoped_restore (&gdb_stdout, str_file);
> diff --git a/gdb/ui-out.c b/gdb/ui-out.c
> index 9fe72ddf3b..34857162b4 100644
> --- a/gdb/ui-out.c
> +++ b/gdb/ui-out.c
> @@ -619,10 +619,10 @@ ui_out::flush ()
> do_flush ();
> }
>
> -int
> +void
> ui_out::redirect (ui_file *outstream)
> {
> - return do_redirect (outstream);
> + do_redirect (outstream);
> }
>
> /* Test the flags against the mask given. */
> diff --git a/gdb/ui-out.h b/gdb/ui-out.h
> index c7e0efb2e0..9f56866903 100644
> --- a/gdb/ui-out.h
> +++ b/gdb/ui-out.h
> @@ -121,7 +121,7 @@ class ui_out
> void flush ();
>
> /* Redirect the output of a ui_out object temporarily. */
> - int redirect (ui_file *outstream);
> + void redirect (ui_file *outstream);
>
> ui_out_flags test_flags (ui_out_flags mask);
>
> @@ -163,7 +163,7 @@ class ui_out
> ATTRIBUTE_PRINTF (2,0) = 0;
> virtual void do_wrap_hint (const char *identstring) = 0;
> virtual void do_flush () = 0;
> - virtual int do_redirect (struct ui_file * outstream) = 0;
> + virtual void do_redirect (struct ui_file * outstream) = 0;
>
> /* Set as not MI-like by default. It is overridden in subclasses if
> necessary. */
> diff --git a/gdb/utils.c b/gdb/utils.c
> index 787e0e37b9..d4be28a34b 100644
> --- a/gdb/utils.c
> +++ b/gdb/utils.c
> @@ -218,8 +218,7 @@ do_ui_out_redirect_pop (void *arg)
> {
> struct ui_out *uiout = (struct ui_out *) arg;
>
> - if (uiout->redirect (NULL) < 0)
> - warning (_("Cannot restore redirection of the current output protocol"));
> + uiout->redirect (NULL);
> }
>
> /* Return a new cleanup that pops the last redirection by ui_out_redirect
>
Otherwise LGTM.
On 2016-12-23 14:44, Luis Machado wrote:
> On 12/22/2016 04:07 PM, Simon Marchi wrote:
>> diff --git a/gdb/cli/cli-logging.c b/gdb/cli/cli-logging.c
>> index 46c23064fc..02e2705310 100644
>> --- a/gdb/cli/cli-logging.c
>> +++ b/gdb/cli/cli-logging.c
>> @@ -130,13 +130,11 @@ set_logging_redirect (char *args, int from_tty,
>> struct cmd_list_element *c)
>>
>> /* There is a former output pushed on the ui_out_redirect stack.
>> We
>> want to replace it by OUTPUT so we must pop the former value
>> - first. We should either do both the pop and push or to do
>> - neither of it. At least do not try to push OUTPUT if the pop
>> - already failed. */
>> + first. Ideally, we should either do both the pop and push or to
>> do
>> + neither of it. */
>
> This comment reads funny. Maybe we should fix it while at it.
>
> "... we should either do both the pop and push or do neither of them.
> "?
Right, now that I re-read it, it does sound funny. Fixed locally to:
/* There is a former output pushed on the ui_out_redirect stack. We
want to replace it by OUTPUT so we must pop the former value
- first. We should either do both the pop and push or to do
- neither of it. At least do not try to push OUTPUT if the pop
- already failed. */
+ first. Ideally, we should either do both the pop and push or do
+ neither of them. */
Thanks!
On 01/03/2017 04:17 PM, Simon Marchi wrote:
> Right, now that I re-read it, it does sound funny. Fixed locally to:
>
> /* There is a former output pushed on the ui_out_redirect stack. We
> want to replace it by OUTPUT so we must pop the former value
> - first. We should either do both the pop and push or to do
> - neither of it. At least do not try to push OUTPUT if the pop
> - already failed. */
> + first. Ideally, we should either do both the pop and push or do
> + neither of them. */
>
> Thanks!
OK with that change.
BTW, also spotted a spurious whitespace after "*" here:
> @@ -163,7 +163,7 @@ class ui_out
> ATTRIBUTE_PRINTF (2,0) = 0;
> virtual void do_wrap_hint (const char *identstring) = 0;
> virtual void do_flush () = 0;
> - virtual int do_redirect (struct ui_file * outstream) = 0;
> + virtual void do_redirect (struct ui_file * outstream) = 0;
Thanks,
Pedro Alves
On 2017-01-10 10:50, Pedro Alves wrote:
> On 01/03/2017 04:17 PM, Simon Marchi wrote:
>
>> Right, now that I re-read it, it does sound funny. Fixed locally to:
>>
>> /* There is a former output pushed on the ui_out_redirect stack.
>> We
>> want to replace it by OUTPUT so we must pop the former value
>> - first. We should either do both the pop and push or to do
>> - neither of it. At least do not try to push OUTPUT if the pop
>> - already failed. */
>> + first. Ideally, we should either do both the pop and push or do
>> + neither of them. */
>>
>> Thanks!
>
> OK with that change.
>
> BTW, also spotted a spurious whitespace after "*" here:
>
> > @@ -163,7 +163,7 @@ class ui_out
> > ATTRIBUTE_PRINTF (2,0) = 0;
> > virtual void do_wrap_hint (const char *identstring) = 0;
> > virtual void do_flush () = 0;
> > - virtual int do_redirect (struct ui_file * outstream) = 0;
> > + virtual void do_redirect (struct ui_file * outstream) = 0;
>
> Thanks,
> Pedro Alves
Oh, right, same in cli-out.h and mi-out.h. I fixed them all and pushed,
thanks.
@@ -227,15 +227,13 @@ cli_ui_out::do_flush ()
and make it therefore active. OUTSTREAM as NULL will pop the last pushed
output stream; it is an internal error if it does not exist. */
-int
+void
cli_ui_out::do_redirect (ui_file *outstream)
{
if (outstream != NULL)
m_streams.push_back (outstream);
else
m_streams.pop_back ();
-
- return 0;
}
/* local functions */
@@ -60,7 +60,7 @@ protected:
ATTRIBUTE_PRINTF (2,0);
virtual void do_wrap_hint (const char *identstring) override;
virtual void do_flush () override;
- virtual int do_redirect (struct ui_file * outstream) override;
+ virtual void do_redirect (struct ui_file * outstream) override;
bool suppress_output ()
{ return m_suppress_output; }
@@ -130,13 +130,11 @@ set_logging_redirect (char *args, int from_tty, struct cmd_list_element *c)
/* There is a former output pushed on the ui_out_redirect stack. We
want to replace it by OUTPUT so we must pop the former value
- first. We should either do both the pop and push or to do
- neither of it. At least do not try to push OUTPUT if the pop
- already failed. */
+ first. Ideally, we should either do both the pop and push or to do
+ neither of it. */
- if (uiout->redirect (NULL) < 0
- || uiout->redirect (output) < 0)
- warning (_("Current output protocol does not support redirection"));
+ uiout->redirect (NULL);
+ uiout->redirect (output);
do_cleanups (cleanups);
}
@@ -246,10 +244,7 @@ handle_redirections (int from_tty)
/* Don't do the redirect for MI, it confuses MI's ui-out scheme. */
if (!current_uiout->is_mi_like_p ())
- {
- if (current_uiout->redirect (output) < 0)
- warning (_("Current output protocol does not support redirection"));
- }
+ current_uiout->redirect (output);
}
static void
@@ -531,10 +531,8 @@ ioscm_with_output_to_port_worker (SCM port, SCM thunk, enum oport oport,
gdb_stderr = port_file;
else
{
- if (current_uiout->redirect (port_file) < 0)
- warning (_("Current output protocol does not support redirection"));
- else
- make_cleanup_ui_out_redirect_pop (current_uiout);
+ current_uiout->redirect (port_file);
+ make_cleanup_ui_out_redirect_pop (current_uiout);
gdb_stdout = port_file;
}
@@ -170,15 +170,13 @@ mi_ui_out::do_flush ()
gdb_flush (m_streams.back ());
}
-int
+void
mi_ui_out::do_redirect (ui_file *outstream)
{
if (outstream != NULL)
m_streams.push_back (outstream);
else
m_streams.pop_back ();
-
- return 0;
}
void
@@ -67,7 +67,7 @@ protected:
ATTRIBUTE_PRINTF (2,0);
virtual void do_wrap_hint (const char *identstring) override;
virtual void do_flush () override;
- virtual int do_redirect (struct ui_file * outstream) override;
+ virtual void do_redirect (struct ui_file * outstream) override;
virtual bool do_is_mi_like_p () override
{ return true; }
@@ -706,10 +706,8 @@ execute_command_to_string (char *p, int from_tty)
make_cleanup_ui_file_delete (str_file);
- if (current_uiout->redirect (str_file) < 0)
- warning (_("Current output protocol does not support redirection"));
- else
- make_cleanup_ui_out_redirect_pop (current_uiout);
+ current_uiout->redirect (str_file);
+ make_cleanup_ui_out_redirect_pop (current_uiout);
scoped_restore save_stdout
= make_scoped_restore (&gdb_stdout, str_file);
@@ -619,10 +619,10 @@ ui_out::flush ()
do_flush ();
}
-int
+void
ui_out::redirect (ui_file *outstream)
{
- return do_redirect (outstream);
+ do_redirect (outstream);
}
/* Test the flags against the mask given. */
@@ -121,7 +121,7 @@ class ui_out
void flush ();
/* Redirect the output of a ui_out object temporarily. */
- int redirect (ui_file *outstream);
+ void redirect (ui_file *outstream);
ui_out_flags test_flags (ui_out_flags mask);
@@ -163,7 +163,7 @@ class ui_out
ATTRIBUTE_PRINTF (2,0) = 0;
virtual void do_wrap_hint (const char *identstring) = 0;
virtual void do_flush () = 0;
- virtual int do_redirect (struct ui_file * outstream) = 0;
+ virtual void do_redirect (struct ui_file * outstream) = 0;
/* Set as not MI-like by default. It is overridden in subclasses if
necessary. */
@@ -218,8 +218,7 @@ do_ui_out_redirect_pop (void *arg)
{
struct ui_out *uiout = (struct ui_out *) arg;
- if (uiout->redirect (NULL) < 0)
- warning (_("Cannot restore redirection of the current output protocol"));
+ uiout->redirect (NULL);
}
/* Return a new cleanup that pops the last redirection by ui_out_redirect