From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <gdb-patches-return-59576-listarch-gdb-patches=sources.redhat.com@sourceware.org>
Received: (qmail 26770 invoked by alias); 29 Oct 2008 16:45:39 -0000
Received: (qmail 26459 invoked by uid 22791); 29 Oct 2008 16:45:37 -0000
X-Spam-Check-By: sourceware.org
Received: from mail.codesourcery.com (HELO mail.codesourcery.com) (65.74.133.4)     by sourceware.org (qpsmtpd/0.31) with ESMTP; Wed, 29 Oct 2008 16:45:31 +0000
Received: (qmail 9162 invoked from network); 29 Oct 2008 16:45:29 -0000
Received: from unknown (HELO orlando.local) (pedro@127.0.0.2)   by mail.codesourcery.com with ESMTPA; 29 Oct 2008 16:45:29 -0000
From: Pedro Alves <pedro@codesourcery.com>
To: gdb-patches@sourceware.org,  tromey@redhat.com
Subject: Re: RFA: cleanups -vs- gdb_fopen
Date: Wed, 29 Oct 2008 17:39:00 -0000
User-Agent: KMail/1.9.9
References: <m3mygomvpj.fsf@fleche.redhat.com>
In-Reply-To: <m3mygomvpj.fsf@fleche.redhat.com>
MIME-Version: 1.0
Content-Type: text/plain;   charset="iso-8859-1"
Content-Transfer-Encoding: quoted-printable
Content-Disposition: inline
Message-Id: <200810291645.56749.pedro@codesourcery.com>
X-IsSubscribed: yes
Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm
Precedence: bulk
List-Id: <gdb-patches.sourceware.org>
List-Subscribe: <mailto:gdb-patches-subscribe@sourceware.org>
List-Archive: <http://sourceware.org/ml/gdb-patches/>
List-Post: <mailto:gdb-patches@sourceware.org>
List-Help: <mailto:gdb-patches-help@sourceware.org>, <http://sourceware.org/ml/#faqs>
Sender: gdb-patches-owner@sourceware.org
X-SW-Source: 2008-10/txt/msg00692.txt.bz2

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

This one had me scratch my head a bit:

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

+  cleanups =3D make_cleanup_ui_file_delete (output);

  /* Redirects everything to gdb_stdout while this is running.  */
  if (!logging_redirect)
    {
      output =3D tee_file_new (gdb_stdout, 0, output, 1);
      ^^^^^^                                        ^
      if (output =3D=3D 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.

--=20
Pedro Alves