This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: [Patch Darwin] minor fixups in section name reading.
- From: Tristan Gingold <gingold at adacore dot com>
- To: Iain Sandoe <developer at sandoe-acoustics dot co dot uk>
- Cc: binutils at sourceware dot org
- Date: Fri, 9 Dec 2011 09:31:05 +0100
- Subject: Re: [Patch Darwin] minor fixups in section name reading.
- References: <3AB996CD-6127-40B0-BC60-48084B1DBCF6@sandoe-acoustics.co.uk>
On Dec 8, 2011, at 3:18 PM, Iain Sandoe wrote:
> this corrects a couple of trivial typos in bfd_mach_o_read_section_32/64.
> It also caters for the fact that segment and section names may fill the on-disk field which is not zero-terminated.
> So one cannot use them directly as strings.
Thank you for working on Mach-O.
See my comments.
(I think it is easier to review patches if they are inline).
Tristan.
> Index: bfd/mach-o.c
> ===================================================================
> RCS file: /cvs/src/src/bfd/mach-o.c,v
> retrieving revision 1.72
> diff -u -p -r1.72 mach-o.c
> --- bfd/mach-o.c 7 Dec 2011 10:09:22 -0000 1.72
> +++ bfd/mach-o.c 8 Dec 2011 14:12:55 -0000
> @@ -1667,9 +1667,16 @@ bfd_mach_o_make_bfd_section (bfd *abfd,
> {
> const char *sname;
> flagword flags;
> + char seg[BFD_MACH_O_SEGNAME_SIZE+1];
> + char sec[BFD_MACH_O_SECTNAME_SIZE+1];
> +
> + memset (seg, 0, sizeof(seg));
> + strncpy (seg, (char *)segname, BFD_MACH_O_SEGNAME_SIZE);
> + memset (sec, 0, sizeof(sec));
> + strncpy (sec, (char *)sectname, BFD_MACH_O_SECTNAME_SIZE);
>
> bfd_mach_o_convert_section_name_to_bfd
> - (abfd, (const char *)segname, (const char *)sectname, &sname, &flags);
> + (abfd, (const char *)seg, (const char *)sec, &sname, &flags);
> if (sname == NULL)
> return NULL;
This is not necessary as bfd_mach_o_convert_section_name_to_bfd is documented as taking non nul terminated strings. If there is a bug, it is in the later function.
> @@ -1698,7 +1705,7 @@ bfd_mach_o_read_section_32 (bfd *abfd,
> memcpy (section->segname, raw.segname, sizeof (raw.segname));
> section->segname[BFD_MACH_O_SEGNAME_SIZE] = 0;
> memcpy (section->sectname, raw.sectname, sizeof (raw.sectname));
> - section->segname[BFD_MACH_O_SECTNAME_SIZE] = 0;
> + section->sectname[BFD_MACH_O_SECTNAME_SIZE] = 0;
> section->addr = bfd_h_get_32 (abfd, raw.addr);
> section->size = bfd_h_get_32 (abfd, raw.size);
> section->offset = bfd_h_get_32 (abfd, raw.offset);
> @@ -1737,7 +1744,7 @@ bfd_mach_o_read_section_64 (bfd *abfd,
> memcpy (section->segname, raw.segname, sizeof (raw.segname));
> section->segname[BFD_MACH_O_SEGNAME_SIZE] = 0;
> memcpy (section->sectname, raw.sectname, sizeof (raw.sectname));
> - section->segname[BFD_MACH_O_SECTNAME_SIZE] = 0;
> + section->sectname[BFD_MACH_O_SECTNAME_SIZE] = 0;
> section->addr = bfd_h_get_64 (abfd, raw.addr);
> section->size = bfd_h_get_64 (abfd, raw.size);
> section->offset = bfd_h_get_32 (abfd, raw.offset);
Good catch. You can commit this part.
Thanks,
Tristan.