From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 25097 invoked by alias); 29 Mar 2012 18:21:21 -0000 Received: (qmail 24942 invoked by uid 22791); 29 Mar 2012 18:21:19 -0000 X-SWARE-Spam-Status: No, hits=-1.2 required=5.0 tests=AWL,BAYES_00,KAM_STOCKGEN X-Spam-Check-By: sourceware.org Received: from rock.gnat.com (HELO rock.gnat.com) (205.232.38.15) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Thu, 29 Mar 2012 18:21:06 +0000 Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id EB6301C6B8E; Thu, 29 Mar 2012 14:21:05 -0400 (EDT) Received: from rock.gnat.com ([127.0.0.1]) by localhost (rock.gnat.com [127.0.0.1]) (amavisd-new, port 10024) with LMTP id Kn9RVyv6yKuF; Thu, 29 Mar 2012 14:21:05 -0400 (EDT) Received: from joel.gnat.com (localhost.localdomain [127.0.0.1]) by rock.gnat.com (Postfix) with ESMTP id AFFE21C6B88; Thu, 29 Mar 2012 14:21:05 -0400 (EDT) Received: by joel.gnat.com (Postfix, from userid 1000) id 8F65B145616; Thu, 29 Mar 2012 11:21:02 -0700 (PDT) Date: Thu, 29 Mar 2012 18:21:00 -0000 From: Joel Brobecker To: Roland Schwingel Cc: gdb-patches@sourceware.org Subject: Re: [PATCH v3] Add dll trampoline code handling for windows 64bit Message-ID: <20120329182102.GO2701@adacore.com> References: <4F630E3C.8010006@onevision.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <4F630E3C.8010006@onevision.com> User-Agent: Mutt/1.5.20 (2009-06-14) 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: 2012-03/txt/msg01009.txt.bz2 Hi Roland, > 2012-03-16 Roland Schwingel > > * amd64-windows-tdep.c: #include "frame.h". > (amd64_windows_skip_trampoline_code): New function. > (amd64_windows_init_abi): Add trampoline registration. No one reviewed þhe patch as far as I can tell, I am sorry about that. As a general guideline, it's OK to ping us every week or two... I think the code looks pretty good, and you can commit, with a couple of very minor almost-nitpicky little comments (a apologize for asking for your forgiveness)... Hmmm, I actually cannot find any copyright assignment for you on file. Do you have one? Or is this done on behalf of your employer who might have one? Please let me know... We can give you write- after-approval priviledges as soon as we have confirmed that you do have an assignment on file. > + /* Get address of function pointer at end of pc. */ > + CORE_ADDR indirect_addr = pc + offset + 6; I think it would be useful to explain where the magic constant "6" comes from... I'd almost write the expression "pc + 6 + offset". > + struct minimal_symbol *indsym = > + indirect_addr ? lookup_minimal_symbol_by_pc (indirect_addr) : 0; > + const char *symname = indsym ? SYMBOL_LINKAGE_NAME (indsym) : 0; I'd rather you used NULL instead of 0, even if we're pretty much guaranteed that NULL will always be zero. That's really nit-picky, but it seems clearer that way, at least for me. > + destination = Trailing space there? > + /* register trampoline handling code. */ Sentences need to start with a capital letter... -- Joel