Re: [PATCH 1/4] Add debug redirect option

[Tom Tromey <tom at tromey dot com>]

>>>>> "Alan" == Alan Hayward <Alan.Hayward@arm.com> writes:

Alan> Add the option to redirect debug output seperately to normal
Alan> output, using the cli command:

Alan>   set logging debugredirect on

[...]

Alan> [
Alan> An alternative would be to add an extra option to the original
Alan> redirect command:
Alan>   set logging redirect on|off|debug
Alan> This method is less flexible, but may be more intuative - setting to
Alan> debug would cause just debug to be redirected.  It would break
Alan> existing scripts that enable using "set logging redirect 1" instead
Alan> of "set logging redirect on".
Alan> ]

I think the choice you implemented here is fine.

Alan> [ I'm also a little unsure if I have the "release" logic correct. ]

One way to improve this code would be to change tee_file to use
ui_file_up.  That way the call to the constructor wouldn't need to use
release(), just std::move.  See the appended.

Tom

diff --git a/gdb/cli/cli-logging.c b/gdb/cli/cli-logging.c
index 3a5e14de3c7..8e34bfc12bc 100644
--- a/gdb/cli/cli-logging.c
+++ b/gdb/cli/cli-logging.c
@@ -97,13 +97,7 @@ make_logging_output (ui_file *curr_output, ui_file_up logfile,
   if (logging_redirect)
     return logfile.release ();
   else
-    {
-      /* Note that the "tee" takes ownership of the log file.  */
-      ui_file *out = new tee_file (curr_output, false,
-				   logfile.get (), true);
-      logfile.release ();
-      return out;
-    }
+    return new tee_file (curr_output, std::move (logfile));
 }
 
 /* This is a helper for the `set logging' command.  */
diff --git a/gdb/ui-file.c b/gdb/ui-file.c
index 77f6b31ce4b..f18738af624 100644
--- a/gdb/ui-file.c
+++ b/gdb/ui-file.c
@@ -283,20 +283,13 @@ stderr_file::stderr_file (FILE *stream)
 
 
 
-tee_file::tee_file (ui_file *one, bool close_one,
-		    ui_file *two, bool close_two)
+tee_file::tee_file (ui_file *one, ui_file_up &&two)
   : m_one (one),
-    m_two (two),
-    m_close_one (close_one),
-    m_close_two (close_two)
+    m_two (std::move (two))
 {}
 
 tee_file::~tee_file ()
 {
-  if (m_close_one)
-    delete m_one;
-  if (m_close_two)
-    delete m_two;
 }
 
 void
diff --git a/gdb/ui-file.h b/gdb/ui-file.h
index 6e6ca1c9cdc..a932f215416 100644
--- a/gdb/ui-file.h
+++ b/gdb/ui-file.h
@@ -243,11 +243,9 @@ public:
 class tee_file : public ui_file
 {
 public:
-  /* Create a file which writes to both ONE and TWO.  CLOSE_ONE and
-     CLOSE_TWO indicate whether the original files should be closed
-     when the new file is closed.  */
-  tee_file (ui_file *one, bool close_one,
-	    ui_file *two, bool close_two);
+  /* Create a file which writes to both ONE and TWO.  ONE will remain
+     open when this object is destroyed; but TWO will be closed.  */
+  tee_file (ui_file *one, ui_file_up &&two);
   ~tee_file () override;
 
   void write (const char *buf, long length_buf) override;
@@ -258,10 +256,9 @@ public:
   void flush () override;
 
 private:
-  /* The two underlying ui_files, and whether they should each be
-     closed on destruction.  */
-  ui_file *m_one, *m_two;
-  bool m_close_one, m_close_two;
+  /* The two underlying ui_files.  */
+  ui_file *m_one;
+  ui_file_up m_two;
 };
 
 #endif