From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 1029 invoked by alias); 2 Jan 2004 17:51:39 -0000 Mailing-List: contact gdb-patches-help@sources.redhat.com; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sources.redhat.com Received: (qmail 1022 invoked from network); 2 Jan 2004 17:51:38 -0000 Received: from unknown (HELO nevyn.them.org) (66.93.172.17) by sources.redhat.com with SMTP; 2 Jan 2004 17:51:38 -0000 Received: from drow by nevyn.them.org with local (Exim 4.30 #1 (Debian)) id 1AcTSU-00035R-Gc for ; Fri, 02 Jan 2004 12:51:38 -0500 Date: Fri, 02 Jan 2004 17:51:00 -0000 From: Daniel Jacobowitz To: gdb-patches@sources.redhat.com Subject: Re: [RFA]: Fix do_cleanups if oldchain is NULL Message-ID: <20040102175138.GB11549@nevyn.them.org> Mail-Followup-To: gdb-patches@sources.redhat.com References: <3FE0C502.7020408@redhat.com> <3FF59719.7020908@gnu.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <3FF59719.7020908@gnu.org> User-Agent: Mutt/1.5.1i X-SW-Source: 2004-01/txt/msg00029.txt.bz2 On Fri, Jan 02, 2004 at 11:06:49AM -0500, Andrew Cagney wrote: > >I recently solved a bug on the ia64 concerning cleanups. What was > >happening was that a cleanup list was being re-initialized to NULL inside > >a loop and later do_cleanups() was called. This caused the entire cleanup > >list to be run because the design is to run the list until the passed in > >cleanup is reached. This caused other errors when the stream being used > >was deleted, etc... > > > >This patch adds a check to do_my_cleanups() so no cleanups will be > >performed if the passed in chain is NULL. > > > >Ok to commit? > > (hmm, no one thought to review this while I was on hols :-() Sorry, I was holding off on reviewing other people's patches during my vacation so that I could catch up on my own. > I think the bug is in the calling code, and not utils.c. That patch > unfortunatly makes a fundamental change to the core of the cleanup code > and there's no easy way of demonstrating that other callers aren't > assuming that NULL implies do all cleanups. How about making NULL an internal error then, as below? I think we should either do that or document its behavior, and it seems accident-prone. -- Daniel Jacobowitz MontaVista Software Debian GNU/Linux Developer 2003-01-02 Daniel Jacobowitz * utils.c (do_my_cleanups): Raise an internal error if the old_chain is NULL. Index: utils.c =================================================================== RCS file: /cvs/src/src/gdb/utils.c,v retrieving revision 1.111 diff -u -p -r1.111 utils.c --- utils.c 2 Jan 2004 17:35:01 -0000 1.111 +++ utils.c 2 Jan 2004 17:49:37 -0000 @@ -1,8 +1,8 @@ /* General utility routines for GDB, the GNU debugger. Copyright 1986, 1988, 1989, 1990, 1991, 1992, 1993, 1994, 1995, - 1996, 1997, 1998, 1999, 2000, 2001, 2002, 2003 Free Software - Foundation, Inc. + 1996, 1997, 1998, 1999, 2000, 2001, 2002, 2003, 2004 + Free Software Foundation, Inc. This file is part of GDB. @@ -318,6 +318,11 @@ do_my_cleanups (struct cleanup **pmy_cha struct cleanup *old_chain) { struct cleanup *ptr; + + if (old_chain == NULL) + internal_error (__FILE__, __LINE__, + "NULL chain passed to do_my_cleanups"); + while ((ptr = *pmy_chain) != old_chain) { *pmy_chain = ptr->next; /* Do this first incase recursion */