[PATCH] Core file build-ids
Nick Clifton
nickc@redhat.com
Fri Jun 7 16:16:00 GMT 2019
Hi Keith,
I do have a couple of comments on this patch:
> +void> +_bfd_elf_core_find_build_id (bfd *templ, bfd_vma offset)> +{> + return (*get_elf_backend_data (templ)->elf_backend_core_find_build_id)> + (templ, offset);
I would recommend being a little bit paranoid here and checking
that templ is an elf format bfd before attempting to access the
backend data.
> - return _bfd_elf_make_section_from_phdr (abfd, hdr, hdr_index, "load");
> + if (! _bfd_elf_make_section_from_phdr (abfd, hdr, hdr_index, "load"))
> + return FALSE;
> + if (bfd_get_format (abfd) == bfd_core && abfd->build_id == NULL)
> + _bfd_elf_core_find_build_id (abfd, hdr->p_offset);
> + return TRUE;
If _bfd_elf_core_find_build_id can fail (see above and below) then
it would be worth checking the return value for an error result.
> +/* Attempt to find a build-id in a core file from the core file BFD.
> + OFFSET is the file offset to a PT_LOAD segment that may contain
> + the build-id note. */
> +
> +void
> +NAME(_bfd_elf,core_find_build_id)
Given that things can wrong when attempting to find a build_id, I think
that this function should return a bfd_boolean. I appreciate that the
caller could just check abfd->build_id != NULL, (assuming that it was NULL
to start with), but I think that it is cleaner that the function itself
should tell its caller whether ot not it succeeded.
> + Elf_External_Ehdr x_ehdr; /* Elf file header, external form */
> + Elf_Internal_Ehdr i_ehdr; /* Elf file header, internal form */
Minor formatting nit: comments should end with a full stop and two spaces.
Cheers
Nick
More information about the Binutils
mailing list