Re: [Patch Darwin] minor fixups in section name reading.

[Tristan Gingold <gingold at adacore dot com>]

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.