[PATCH,V6 06/10] bfd: linker: merge .ctf_frame sections

Nick Clifton nickc@redhat.com
Mon Aug 15 13:02:33 GMT 2022

Hi Indu,

> +ctf_frame_decoder_mark_func_deleted (struct ctf_frame_dec_info *cfd_info,
> +				     unsigned int func_idx)
> +{
> +  BFD_ASSERT (func_idx < cfd_info->cfd_fde_count);
> +  cfd_info->cfd_func_bfdinfo[func_idx].func_deleted_p = true;

Just for the record, I am not a fan of assertions inside library functions.
I believe that libraries should let their users decide what to do when there
is a problem, rather than unilaterally calling abort.  (The better approach
in my opinion is to return an informative error message to the caller).

That said there are plenty of other places in the BFD library where assertions
are used, so I am not going to complain about this.  I just wanted to make
sure that you knew of my feelings on this issue.

> +/* Try to parse .ctf_frame section SEC, which belongs to ABFD.  Store the
> +   information in the section's sec_info field on success.  COOKIE
> +   describes the relocations in SEC.  */
> +
> +void
> +_bfd_elf_parse_ctf_frame (bfd *abfd, struct bfd_link_info *info,
> +			  asection *sec, struct elf_reloc_cookie *cookie)

It seems to me that this function really ought to return a bool,
indicating success or failure.

> +  /* Read the ctf frame unwind information from abfd.  */
> +  if (!bfd_malloc_and_get_section (abfd, sec, &ctfbuf))
> +    goto fail_no_free;

The name of this label seems rather ironic, given that ...

> +fail_no_free:
> +  _bfd_error_handler
> +   (_("error in %pB(%pA); no .ctf_frame will be created"),
> +    abfd, sec);
> +success:
> +  free (ctfbuf);

... it falls through into the free().


More information about the Binutils mailing list