Hi Danny,
I've tried to tackle the conditional compilation part, and would like
your renewed input on it, I may have missed some points. Naming for
instance.
Naming was mostly OK. There were two problems.
pe_print_compressed_pdata() should really be called
pe_print_ce_compressed_pdata() since it assumes
_IMAGE_CE_RUNTIME_FUNCTION_ENTRY formatted data right ? One day someone
might want to write a MIPS version of this function, so we will need a
Secondly you did not provide a default definition of bfd_pe_print_pdata,
so the PE ports which do not use this new feature will not build. Try
configuring a binutils build with "--enable-targets=all" to see this
happening.
Here are two other things however which I think you should also fix:
+static int symcount=0;
+static asymbol **
+slurp_symtab (bfd *abfd)
Communicating this information via a static is a bit naughty. It would
be cleaner to return a structure containing the symbol table pointer and
the symbol count. This matters even more here:
+static const char *
+my_symbol_for_address(bfd *abfd, bfd_vma func)
+{
+ static asymbol **syms = 0;
+ int i;
+
+ if (syms == 0)
+ syms = slurp_symtab (abfd);
This means that a symbol table will only be read once, even if we are
displaying the pe pdata for more than one file. (Something that objdump
can do). You need to cache the bfd pointer for the symbol table stored
in 'syms' and check that on entry to my_symbol_for_address. Or a
cleaner method would be to have some way to release the loaded symbol
table at the end of pe_print_compressed_pdata so that a following call
to pe_print_compressed_pdata will load a new symbol table.
Index: libcoff.h
===================================================================
--- libcoff.h (revision 1151)
+++ libcoff.h (working copy)
@@ -802,6 +802,8 @@
bfd_boolean (*_bfd_coff_final_link_postscript)
(bfd *, struct coff_final_link_info *);
+ bfd_boolean (*_bfd_coff_print_pdata)
+ (bfd *, void *);
} bfd_coff_backend_data;
#define coff_backend_info(abfd) \
@@ -934,3 +936,7 @@
#define bfd_coff_final_link_postscript(a,p) \
((coff_backend_info (a)->_bfd_coff_final_link_postscript) (a, p))
+#define bfd_coff_have_print_pdata(a) \
+ (coff_backend_info (a)->_bfd_coff_print_pdata)
+#define bfd_coff_print_pdata(a,p) \
+ ((coff_backend_info (a)->_bfd_coff_print_pdata) (a, p))
This is wrong. The libcoff.h file is a generated file. You should
patch the source file that is used to generate these parts of libcoff.h
(in this case coffcode.h) and then either provide a patch for libcoff.h
as well, or else add a small note in the patch submission saying that
the constructed files inside the bfd/ source directory need to be
regenerated.
Apart from that and some minor formatting issues, the patch looks fine.
Cheers
Nick
_______________________________________________
bug-binutils mailing list
bug-binutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-binutils
