Re: [patch] Reading coff-pe-read files

[Joel Brobecker <brobecker@adacore.com>]

> Hmm, at home it isn't hard to do this, but at office I have to fight with 
> Lotus. I'll see what can do.

Thanks for doing that. Perhaps, if Lotus doesn't muck the contents
of your emails, one solution is to inline the patch inside the email
body rather than as an attachment. If, as I fear, Lotus does things
like breaking lines, etc, then the current approach at least allows
us to receive the patch intact, which is the most important. Anyway,
all this monologue just to say: Do your best :).

> > Can you also provide a ChangeLog entry when submitting patches?
> 
> I did in my first mail. I thought there is no new one necessary.

Hmmm, I thought I double-checked before mentioning it, sorry.

> 2009-01-08  Kai Tietz  <kai.tietz@onevision.com>
> 
>         * coff-pe-read.c (read_pe_exported_syms): Enable read of PE+ 
> export directory.

Does "PE+" mean PE for 64bit?

> > > +  int be64 = 0;
> > > +  int be32 = 0;
> > 
> > Would you mind explaining what "be" stands for?
> 
> the "be" from "to be, or not to be" ;)

In that case, I'd really like to use a more meaningful name.
How about pe32_p and pe64_p? Or is_pe32 and is_pe64?

> > > +  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.
[...]
> So as you can see, the entries for ImageBase, SizeOfStackReserve, 
> SizeOfStackCommit,SizeOfHeapReserve.and SizeOfHeapCommit have 64-bit sizes 
> in PE+.
> For PE+ there is no member BaseOfData present anymore. So the delta is 5 * 
> (8 - 4 /* The diff of sizes for those members */) - 4 /* Because there is 
> no BaseOfData member anymore */. So I came to the diff of 16 bytes.

OK - I understand, now. Thanks!

I think that it would be clearer to use 108 in this case than
"92 + 16", because 92 in the PE+ case doesn't really mean anything,
does it?

> > Same here.
> 
> The same reason. Structure IMAGE_DATA_DIRECTORY hasn't changed in sizes, 
> therefore just the delta is necessary.

I also suggest the same as above, if you agree.

> AFAIC I have no write access (or write permissions) to gbd tree. I have 
> already an account and write permissions for binutils.

Given that this patch qualifies as a good patch (or will qualify as soon
as we agree on one version of it), you are eligible for receiving "Write
After Approval" priviledges. What you need to do next, in parallel to
this discussion, is ask overseers to adjust your priviledges.  This is
what the account request on sourceware says about your case:

    Note that if you already have an account on sourceware.org or
    gcc.gnu.org for CVS or Subversion write access, then do not use this
    form. Instead send an email to the overseers mail account at this site
    telling what project you want write access to and who approved that
    access.

You can list me as the approver.

-- 
Joel