This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: [patch]: dump of PE+ x64 pdata section
2009/4/2 Dave Korn <dave.korn.cygwin@googlemail.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
>
>
>
Dave,
ok I'll adjust those stuff. Should I adjust the ce function also,
becaue here a lot of those straying whitespaces are coming from.
I want to add some testcases, when I completed with the next patch
which dumps .xdata, too.
Cheers,
Kai
--
| (\_/) This is Bunny. Copy and paste
| (='.'=) Bunny into your signature to help
| (")_(") him gain world domination