Change return type of ui_out redirect to void

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

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.