Re: [PATCH 2/3] Don't let tee_file own a stream

[Simon Marchi <simark@simark.ca>]

On 11/17/22 13:29, Tom Tromey via Gdb-patches wrote:
> Right now, tee_file owns the second stream it writes to.  This is done
> for the convenience of the users.  In a subsequent patch, this will no
> longer be convenient, so this patch moves the responsibility for
> ownership to the users of tee_file.

I think the idea makes sense, from a software design / decoupling point
of view.

> diff --git a/gdb/mi/mi-interp.c b/gdb/mi/mi-interp.c
> index af242208a0b..482166ea3a5 100644
> --- a/gdb/mi/mi-interp.c
> +++ b/gdb/mi/mi-interp.c
> @@ -1275,21 +1275,16 @@ mi_interp::set_logging (ui_file_up logfile, bool logging_redirect,
>      {
>        mi->saved_raw_stdout = mi->raw_stdout;
>  
> -      /* If something is being redirected, then grab logfile.  */
> -      ui_file *logfile_p = nullptr;
> -      if (logging_redirect || debug_redirect)
> -	{
> -	  logfile_p = logfile.get ();
> -	  mi->saved_raw_file_to_delete = logfile_p;
> -	}
> +      ui_file *logfile_p = logfile.get ();
> +      mi->saved_raw_file_to_delete = logfile.release ();
>  
>        /* If something is not being redirected, then a tee containing both the
>  	 logfile and stdout.  */
>        ui_file *tee = nullptr;
>        if (!logging_redirect || !debug_redirect)
>  	{
> -	  tee = new tee_file (mi->raw_stdout, std::move (logfile));
> -	  mi->saved_raw_file_to_delete = tee;
> +	  tee = new tee_file (mi->raw_stdout, logfile_p);
> +	  mi->saved_tee_to_delete = tee;
>  	}
>  
>        mi->raw_stdout = logging_redirect ? logfile_p : tee;
> @@ -1297,9 +1292,11 @@ mi_interp::set_logging (ui_file_up logfile, bool logging_redirect,
>    else
>      {
>        delete mi->saved_raw_file_to_delete;
> +      delete mi->saved_tee_to_delete;
>        mi->raw_stdout = mi->saved_raw_stdout;
>        mi->saved_raw_stdout = nullptr;
>        mi->saved_raw_file_to_delete = nullptr;
> +      mi->saved_tee_to_delete = nullptr;

Should the two _to_delete ones be ui_file_ups?

Otherwise, everything related to logging / redirection confuse me, so I
can't really comment on the change itself.

Simon