Re: [PATCH v4] Add dll trampoline code handling for windows 64bit

[Pedro Alves <palves at redhat dot com>]

On 04/02/2012 03:53 PM, Roland Schwingel wrote:

>> Please send an updated patch, so we have in the archives the exact
>> patch as what is
>> checked in, and in case some other maintainer wants to take a look,
>> best have him look at
>> the refreshed patch.   In fact, if you had sent it already in that
>> email, there'd have been
>> no extra noise, right?  ;-)
> Regarding the noise right. But not regarding generating additional work. 


Sorry, I can't be sympathetic to that.  You would be making the changes anyway.
I can't believe that pasting a patch at the end of an email is extra work by
any valid measure.  What's real extra work is someone reading an out of
date patch, and trying to figure out from several messages in a thread what
would be the final state of the patch.

> 2012-04-02  Roland Schwingel <roland.schwingel@onevision.com>

                              ^

Should be two spaces after your name.

> 
>          * amd64-windows-tdep.c: #include "frame.h".
>          (amd64_windows_skip_trampoline_code): New function.
>          (amd64_windows_init_abi): Add trampoline registration.


On 04/02/2012 03:53 PM, Roland Schwingel wrote:
> +/* Check win64 DLL jmp trampolines and find jump destination.  */

The correct spelling is "Win64" capitalized.

>  static void
>  amd64_windows_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
>  {
>    struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
>
> +  /* Register trampoline handling code.  */
> +  set_gdbarch_skip_trampoline_code (gdbarch, amd64_windows_skip_trampoline_code);

A nit, but it'd be cleaner/clearer to put this after the amd64_init_abi call, or
better, near the end of the function, after set_gdbarch_skip_main_prologue.  The current
code reads "initialize the base arch, then install overrides.".  This new call
here breaks that flow.

> +
>    amd64_init_abi (info, gdbarch);
>
>    /* On Windows, "long"s are only 32bit.  */


Having once written the equivalent arm-wince-tdep.c:arm_pe_skip_trampoline_code
for ARM WinCE, this generally looks good to me too.

-- 
Pedro Alves