This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: [patch]: dump of PE+ x64 pdata section
- From: Dave Korn <dave dot korn dot cygwin at googlemail dot com>
- To: Kai Tietz <ktietz70 at googlemail dot com>
- Cc: Binutils <binutils at sourceware dot org>
- Date: Thu, 02 Apr 2009 18:10:12 +0100
- Subject: Re: [patch]: dump of PE+ x64 pdata section
- References: <90baa01f0904020835p35a01970p1d140da8b5b6b0dc@mail.gmail.com>
Hi Kai, here's a few style/formatting nit picks for you!
-#ifndef bfd_pe_print_pdata
-#define bfd_pe_print_pdata NULL
-#endif
+#undef bfd_pe_print_pdata
+#define bfd_pe_print_pdata _bfd_pep_print_x64_pdata
Move that extra space after "bfd_pe_print_pdata" to fix alignment.
+ struct sym_cache sym_cache = {0, 0} ;
Another stray space, delete.
+ _("Warning, .pdata section size (%ld) is not a multiple of %d\n"),
That should probably begin "warning:" like other warnings. You can
capitalise "warning" if you like, there already appear to be some messages
each way, but let's keep to the standard colon-as-separator format.
+ if (! bfd_malloc_and_get_section (abfd, section, &data))
Coding standard says unary operators (!-~&*) aren't followed by a space IIRC.
+ begin_addr = bfd_get_32 (abfd, data + i );
+ end_addr = bfd_get_32 (abfd, data + i + 4);
More stray spaces.
+ fprintf_vma (file, i + section->vma); fprintf (file, ":\t");
+ fprintf_vma (file, begin_addr); fputc (' ', file);
+ fprintf_vma (file, end_addr); fputc (' ', file);
Don't concatenate statements on one line please.
+ cleanup_syms (& sym_cache);
Again, no space after unary operator.
> * libpei.h (_bfd_pep_print_x64_pdata): New.
> * peXXigen.c (_bfd_pep_print_x64_pdata): New.
That looks just a little bit confusing. The entry for libpei.h should
probably say "Declare" or "Add prototype" or something like that.
> Tested on i686-pc-cygwin and x86_64-pc-mingw32 without seeing any
> regressions.
Does the code actually get exercised during a testsuite run? If not, would
you add a testcase to ensure that it does?
cheers,
DaveK