Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Joel Brobecker <brobecker@adacore.com>
To: Kai Tietz <Kai.Tietz@onevision.com>
Cc: gdb-patches@sourceware.org
Subject: Re: [patch] Reading coff-pe-read files
Date: Thu, 08 Jan 2009 09:53:00 -0000	[thread overview]
Message-ID: <20090108095336.GP3664@adacore.com> (raw)
In-Reply-To: <OF85EF0545.1B50FC45-ONC1257537.004BFB3B-C1257537.004C1B6A@onevision.de>

Hello Kai,

> I modified my patch, so that it doesn't need a #if clause anymore.

Thanks.  This was an excellent suggestion from Pierre and I'm much
happier with the new patch.

A few comments:

First, do you think you could use a different mime type when attaching
a patch?  I shows up as application/octet-stream which means that my
mailer doesn't treat it as text - This means that when I reply, your
patch isn't quoted automatically as part of the reply.  text/plain would
be great. Some people also use some mailers that do not handle base64
encodings. One of them is Mark Kettenis who's still one of our active
maintainers, particularly on x86/x86_64. You'll have more chances to get
his comments if you use a 7bit ASCII file...

Can you also provide a ChangeLog entry when submitting patches?

> Index: src/gdb/coff-pe-read.c
> ===================================================================
> --- src.orig/gdb/coff-pe-read.c
> +++ src/gdb/coff-pe-read.c
> @@ -191,6 +191,8 @@ read_pe_exported_syms (struct objfile *o
>    unsigned char *expdata, *erva;
>    unsigned long name_rvas, ordinals, nexp, ordbase;
>    char *dll_name;
> +  int be64 = 0;
> +  int be32 = 0;

Would you mind explaining what "be" stands for?

> +  if (be64)
> +    num_entries = pe_get32 (dll, opthdr_ofs + 92 + 16);
> +  else
> +    num_entries = pe_get32 (dll, opthdr_ofs + 92);

Not knowing the PE format all that well, could you explain
these numbers a little? 92 + 16 seems to suggest that the number
of entries is no longer at the beginning of some kind of "section"
but 16 bytes later.

(this is for my education - I tried to find some documentation
about the 64bit PE/COFF format, but the only one I found was from
MS and it's in a docx format that I can't seem to be able to open
with openoffice - I would be very grateful for a PDF)

> -  export_rva = pe_get32 (dll, opthdr_ofs + 96);
> -  export_size = pe_get32 (dll, opthdr_ofs + 100);
> +  if (be64)
> +    {
> +      export_rva = pe_get32 (dll, opthdr_ofs + 96 + 16);
> +      export_size = pe_get32 (dll, opthdr_ofs + 100 + 16);
> +    }
> +  else
> +    {
> +      export_rva = pe_get32 (dll, opthdr_ofs + 96);
> +      export_size = pe_get32 (dll, opthdr_ofs + 100);
> +    }

Same here.

I trust you that the changes are correct (and in any case only
affect x86_64-windows), but I want to understand them a little bit.

In the meantime, do you have write access to the GDB project?
If not, let's also work on that. I see that you have a copyright
assignment already in place, and an account on sourceware.org
as well.

-- 
Joel


  reply	other threads:[~2009-01-08  9:53 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-01-07 13:51 Kai Tietz
2009-01-08  9:53 ` Joel Brobecker [this message]
2009-01-08 10:23   ` Kai Tietz
2009-01-08 11:10     ` Joel Brobecker
2009-01-08 12:53       ` Kai Tietz
2009-01-08 12:58         ` Joel Brobecker
2009-01-08 13:09           ` Joel Brobecker
2009-01-08 13:37             ` Kai Tietz
2009-01-08 20:07           ` Christopher Faylor
2009-01-08 20:55             ` Kai Tietz
2009-01-09  8:58               ` Pedro Alves
2009-01-09  9:33                 ` Kai Tietz
2009-01-08 10:36   ` Pierre Muller
  -- strict thread matches above, loose matches on Subject: below --
2009-01-07 12:27 [RFA/commit] arch-utils.c: Use host_address_to_string when printing function addresses Joel Brobecker
2009-01-07 13:15 ` [patch] Reading coff-pe-read files Kai Tietz
2009-01-07 13:54   ` Pierre Muller

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20090108095336.GP3664@adacore.com \
    --to=brobecker@adacore.com \
    --cc=Kai.Tietz@onevision.com \
    --cc=gdb-patches@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox