From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 2609 invoked by alias); 30 Mar 2012 06:47:34 -0000 Received: (qmail 2584 invoked by uid 22791); 30 Mar 2012 06:47:33 -0000 X-SWARE-Spam-Status: No, hits=0.1 required=5.0 tests=AWL,BAYES_40,KAM_STOCKGEN X-Spam-Check-By: sourceware.org Received: from outdoor.onevision.de (HELO outdoor.onevision.de) (212.77.172.51) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Fri, 30 Mar 2012 06:47:19 +0000 Received: from sanders.onevision.de (moonrace [212.77.172.62]) by outdoor.onevision.de (8.14.3/8.13.7/ROSCH/DDB) with ESMTP id q2U6kpNP027203; Fri, 30 Mar 2012 08:46:56 +0200 Received: from [192.168.5.32] ([192.168.5.32]) by sanders.onevision.de (Lotus Domino Release 8.5.1FP3) with ESMTP id 2012033008464653-6646 ; Fri, 30 Mar 2012 08:46:46 +0200 Message-ID: <4F7556D4.4010404@onevision.com> Date: Fri, 30 Mar 2012 06:47:00 -0000 From: Roland Schwingel User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:10.0.2) Gecko/20120216 Thunderbird/10.0.2 MIME-Version: 1.0 To: gdb-patches@sourceware.org, Joel Brobecker Subject: Re: [PATCH v3] Add dll trampoline code handling for windows 64bit Content-Type: text/plain; charset=ISO-8859-15; format=flowed Content-Transfer-Encoding: quoted-printable 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/msg01038.txt.bz2 Hi Joel, gdb-patches-owner@sourceware.org wrote on 29.03.2012 20:21:02: > > 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 =FEhe 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... Thanks for taking the time and looking at it. > 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)... No problem. > 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. Tom Tromey sent me the necessary informations on march 15th. One day=20 later I sent out the mail to fsf-records@gnu.org but yet have not=20 received *any* response. I asked Tom some days ago and he told me to=20 wait for some more days before I send in a ping. I already got a CVS=20 account on sourceware and I am just awaiting my okays ... > > + /* Get address of function pointer at end of pc. */ > > + CORE_ADDR indirect_addr =3D 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". PC relative instructions always count starting from the end of the=20 instruction itself. pc in this situation points to the beginning of the=20 instruction. The instruction itself is (with operands) 6 bytes long. If=20 you like to I will change the order of addition on commit. > > + struct minimal_symbol *indsym =3D > > + indirect_addr ? lookup_minimal_symbol_by_pc=20 (indirect_addr) : 0; > > + const char *symname =3D 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. Sure I can do that (on commit). I did it the same way as the 32bit code=20 in i386_pe_skip_trampoline_code() does hoping it would be easier for the=20 approver if he sees familiar code ... The other 2 formatting I will also correct on commit. But if you want I=20 am doing a 4th version of my patch with your suggested changes and send=20 it in. Elsewise I will sit and wait until my assignment stuff gets done=20 and commit it later on and avoid unneccessary noise. Roland