From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 57135 invoked by alias); 22 Aug 2019 08:41:34 -0000 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 Received: (qmail 57121 invoked by uid 89); 22 Aug 2019 08:41:34 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-12.2 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.1 spammy= X-HELO: mail-wr1-f67.google.com Received: from mail-wr1-f67.google.com (HELO mail-wr1-f67.google.com) (209.85.221.67) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 22 Aug 2019 08:41:33 +0000 Received: by mail-wr1-f67.google.com with SMTP id r3so4583862wrt.3 for ; Thu, 22 Aug 2019 01:41:32 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=embecosm.com; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=WgPsA/DsigbO0ELhrrsyElPyCBtBE19Vm+zmF7NWg2Y=; b=HUotXeAJkYYW7Zw0r2i6sYZfniArlGWK8V86VL9xPzmDHwcZ5klAlXB2wq/TbhIMFW L5A1BH48SYCrv06eDVdc0fZUrPE8keYL71j827cHFhakbWh8yYEdmVeVih1IKilsSFIm 1xKI6bvDHTUwP8MzSZpNbAx5hERRKgrsIIa7YXE5xUijze8oWgrBSHDkQsZ3/9bkWRXq fblaHeYbGk6a5PH95UUS3UOQmoSXAF5xaWpkdL+e+0HANetzVCYW9JCaAAROJp+0bRIX v+1BaIZE6G4kGsiQmPuwY4+PCiH6FAEAlDXqRwNcKtGn3KLmiSus/sGibZxSofg7EYpn EVew== Return-Path: Received: from localhost (host86-128-12-79.range86-128.btcentralplus.com. [86.128.12.79]) by smtp.gmail.com with ESMTPSA id i93sm34298705wri.57.2019.08.22.01.41.30 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Thu, 22 Aug 2019 01:41:30 -0700 (PDT) Date: Thu, 22 Aug 2019 08:41:00 -0000 From: Andrew Burgess To: Tom Tromey Cc: gdb-patches@sourceware.org Subject: Re: [PATCH v2 2/8] Handle copy relocations Message-ID: <20190822084129.GK6076@embecosm.com> References: <20190801170412.5553-1-tromey@adacore.com> <20190801170412.5553-3-tromey@adacore.com> <20190821153528.GH6076@embecosm.com> <87ftlul4ed.fsf@tromey.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <87ftlul4ed.fsf@tromey.com> X-Fortune: Hackers of the world, unite! X-Editor: GNU Emacs [ http://www.gnu.org/software/emacs ] User-Agent: Mutt/1.9.2 (2017-12-15) X-IsSubscribed: yes X-SW-Source: 2019-08/txt/msg00521.txt.bz2 * Tom Tromey [2019-08-21 13:11:54 -0600]: > >>>>> "Andrew" == Andrew Burgess writes: > > >> +struct minimal_symbol * > >> +lookup_minimal_symbol_linkage (const char *name, struct objfile *objf) > > Andrew> I found the name chosen for this function rather confusing. When > Andrew> comparing to say 'lookup_minimal_symbol_text' which does > Andrew> sort-of-almost the same thing but for text type symbols, I might have > Andrew> been tempted to go with 'lookup_minimal_symbol_data'. Am I missing > Andrew> something behind the choice of function name? > > You aren't missing anything. > > lookup_minimal_symbol_data seems maybe confusing too, because > lookup_minimal_symbol_text loops over all objfiles, but this one > does not. Indeed, and the return type is different. I also notice that lookup_minimal_symbol_text can be restricted to a single objfile, however in that case we _still_ iterate over all object files. So then I thought, surely we can do better, maybe add a single object iterator and have lookup_minimal_symbol_text not walk through all object files.... but then I spotted this condition: if (objf == NULL || objf == objfile || objf == objfile->separate_debug_objfile_backlink) { /* Process object file. */ } So, in this case we have to scan all objfiles in case we have split debug information. Wouldn't this apply to your new function too? I guess I'm thinking that we could merge the core of lookup_minimal_symbol_text and lookup_minimal_symbol_linkage (or _data), and just pass in a comparison function to check for either text or data symbols. > > I guess I don't care all that much (as evidenced by the original choice > of name ;) so if you still like that better, I'll make the change. Meh! I guess my only request would be can the comment at least explain that it's similar to *_text but for data as currently neither the function name nor the comment mention data at all. Thanks, Andrew