From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 14595 invoked by alias); 6 May 2008 14:18:20 -0000 Received: (qmail 14450 invoked by uid 22791); 6 May 2008 14:18:18 -0000 X-Spam-Check-By: sourceware.org Received: from NaN.false.org (HELO nan.false.org) (208.75.86.248) by sourceware.org (qpsmtpd/0.31) with ESMTP; Tue, 06 May 2008 14:17:58 +0000 Received: from nan.false.org (localhost [127.0.0.1]) by nan.false.org (Postfix) with ESMTP id 3E81C9835A; Tue, 6 May 2008 14:17:57 +0000 (GMT) Received: from caradoc.them.org (22.svnf5.xdsl.nauticom.net [209.195.183.55]) by nan.false.org (Postfix) with ESMTP id B956F980F7; Tue, 6 May 2008 14:17:56 +0000 (GMT) Received: from drow by caradoc.them.org with local (Exim 4.69) (envelope-from ) id 1JtNzO-0004io-UR; Tue, 06 May 2008 10:17:54 -0400 Date: Tue, 06 May 2008 15:47:00 -0000 From: Daniel Jacobowitz To: Aleksandar Ristovski Cc: gdb-patches@sourceware.org Subject: Re: [patch] Do not add partial_symbol again and again to the list Message-ID: <20080506141754.GA17275@caradoc.them.org> Mail-Followup-To: Aleksandar Ristovski , gdb-patches@sourceware.org References: <20080211210935.GA31767@caradoc.them.org> <47B0C0F4.4090302@qnx.com> <20080211214750.GA1953@caradoc.them.org> <47B0C7C6.9090605@qnx.com> <20080211223056.GA3833@caradoc.them.org> <47B0CF8A.6080306@qnx.com> <20080211225314.GA5832@caradoc.them.org> <47B27EB1.6030606@qnx.com> <20080503205413.GA22704@caradoc.them.org> <481F58ED.5070207@qnx.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <481F58ED.5070207@qnx.com> User-Agent: Mutt/1.5.17 (2007-12-11) X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2008-05/txt/msg00231.txt.bz2 On Mon, May 05, 2008 at 02:58:53PM -0400, Aleksandar Ristovski wrote: >>> +const void * >>> +bcache (const void *addr, int length, struct bcache *bcache) >>> +{ >>> + return bcache_data (addr, length, bcache); >>> +} >>> + >>> +void * >>> +bcache_added (const void *addr, int length, struct bcache *bcache, >>> + int *added) >> >> It should return a const pointer, like bcache. Also the indentation >> on the second line is too deep. > > bcache_added is more like bcache_data which returns void*. It would make > sense to return void const* but then I would have to change const-ness in > many places (too many: I would leave that for some other patch). This really is not OK. Take a look at the explanation of a bcache: A bcache is a data structure for factoring out duplication in read-only structures. If you modify the pointed-to object then things go wrong: we put duplicates in the cache which is what it's supposed to avoid. And you modify the other copies which share the same bcache entry, corrupting them. If you don't want to fix the constification issues right now, then just call this deprecated_bcache_added and we can fix it later; you're not making it any worse. But this is something you have to understand about the bcache. >> In fact, if lookup_partial_symbol will never find a global symbol >> with an identical name why do we need them for any duplicated symbol, >> even legitimately duplicated things like static functions? > > I don't think static functions are a good example since they would not go > into the global list anyway (see dwarf2read.c, case DW_TAG_subprogram). > > The attached is revised patch with the changes above; additionally, it > calls new add_partial_symbol_to_global_list for all partial symbols that > are being added to the global list. This is not symbol-reader specific, right? There's nothing that makes it safe for DWARF2 but unsafe for stabs? If so, a smaller patch would be do just do this in add_psymbol_to_list if list == &objfile->global_symbols. -- Daniel Jacobowitz CodeSourcery