Re: RFA: cleanups -vs- gdb_fopen

[Pedro Alves <pedro@codesourcery.com>]

On Tuesday 28 October 2008 18:16:24, Tom Tromey wrote:
> @@ -97,6 +98,7 @@ handle_redirections (int from_tty)
>    output = gdb_fopen (logging_filename, logging_overwrite ? "w" : "a");
>    if (output == NULL)
>      perror_with_name (_("set logging"));
> +  cleanups = make_cleanup_ui_file_delete (output);
>  
>    /* Redirects everything to gdb_stdout while this is running.  */
>    if (!logging_redirect)
> @@ -112,6 +114,8 @@ handle_redirections (int from_tty)
>      fprintf_unfiltered (gdb_stdout, "Redirecting output to %s.\n",
>                         logging_filename);
>  
> +  discard_cleanups (cleanups);

This one had me scratch my head a bit:

  output = gdb_fopen (logging_filename, logging_overwrite ? "w" : "a");
  if (output == NULL)
    perror_with_name (_("set logging"));

+  cleanups = make_cleanup_ui_file_delete (output);

  /* Redirects everything to gdb_stdout while this is running.  */
  if (!logging_redirect)
    {
      output = tee_file_new (gdb_stdout, 0, output, 1);
      ^^^^^^                                        ^
      if (output == NULL)
	perror_with_name (_("set logging"));
      if (from_tty)
	fprintf_unfiltered (gdb_stdout, "Copying output to %s.\n",
			    logging_filename);
    }
  else if (from_tty)
    fprintf_unfiltered (gdb_stdout, "Redirecting output to %s.\n",
			logging_filename);

+  discard_cleanups (cleanups);

If an error is thrown after tee_file_new returning a valid file (it
always does currently), you'd run the cleanup on the old `output',
and leave the new `output' leaking.  That can't happen with the current
code, so, your patch looks good to me.

Thanks.

-- 
Pedro Alves