This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: PATCH: support compressed sections in addr2line, objdump, readelf
- From: Nick Clifton <nickc at redhat dot com>
- To: Craig Silverstein <csilvers at google dot com>
- Cc: binutils at sourceware dot org
- Date: Mon, 07 Jul 2008 13:12:18 +0100
- Subject: Re: PATCH: support compressed sections in addr2line, objdump, readelf
- References: <20080622061003.D279F3F303A@localhost>
Hi Craig,
Ian Lance Taylor and Cary Coutant have given the patch a provisional
look-through, without finding any problems. Please give a look and
let me know if you think it's good to commit.
The patch looks pretty good, although there are a few formatting issues:
+/* Extracted from compress.c. */
+bfd_boolean bfd_uncompress_section_contents
+ (bfd_byte **buffer, bfd_size_type *size);
Please put the function name on a separate line after its return type,
rather than following it on the same line. (This is helpful to people
who use emacs).
+/* Read a section into its appropriate place in the dwarf2_debug
+ * struct (indicated by SECTION_BUFFER and SECTION_SIZE). If syms is
+ * not NULL, use bfd_simple_get_relocated_section_contents to read the
+ * section contents, otherwise use bfd_get_section_contents. */
Please do not start comment lines with asterisks. Comments are meant to
be read as straightforward sentences without any funky formatting.
+static bfd_boolean
+read_section (bfd *abfd,
+ const char* section_name, const char* compressed_section_name,
+ asymbol** syms, bfd_uint64_t offset,
+ bfd_byte **section_buffer, unsigned long *section_size)
+{
+ asection *msec;
+ int section_is_compressed = 0;
Why is "section_is_compressed" an int instead of a bfd_boolean ?
+ if (! read_section (unit->abfd, ".debug_str", ".zdebug_str",
+ if (! read_section (abfd, ".debug_abbrev", ".zdebug_abbrev",
+ if (! read_section (abfd, ".debug_line", ".zdebug_line",
Hmm - can the compressed section name always be deduced from the
uncompressed name ? If so then there is no need for the compressed name
to be provided to read_section(), which might help to reduce code bugs
from a mis-typed compressed section name.
+ /* There can be more than one DWARF2 info section in a BFD these
+ days. First handle the easy case there's only one. If
Missing word: "First handle the easy case *when* there's only one."
One final question - how did you test your patch ?
Cheers
Nick