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

[Joel Brobecker <brobecker@adacore.com>]

Hi Roland,

> 2012-03-16  Roland Schwingel <roland.schwingel@onevision.com>
> 
>         * 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