[PATCH v8 07/19] readelf: dump Object Attributes v2

Jan Beulich jbeulich@suse.com
Fri Aug 8 12:42:05 GMT 2025


On 15.07.2025 13:39, Matthieu Longo wrote:
> @@ -19435,6 +19436,320 @@ free_data:

Here you can, btw, see what happens when a label is entirely unindented.
There's at least one such problem in this patch as well.

>    return res;
>  }
>  
> +typedef gas_symbol_t known_tag_v2;
> +typedef struct
> +{
> +  const char *subsec_name;
> +  const known_tag_v2 *known_tags;
> +  const size_t len;

I'd recommend against this one "const".

> +} _known_subsection_v2;

Any particular need for the leading underscore here?

> +static const known_tag_v2 known_tags_aeabi_feature_and_bits[] =
> +{
> +  {"Tag_Feature_BTI", .value = {

See my earlier remark regarding the consistent use of struct field
initializers.

> +static const _known_subsection_v2 *
> +_identify_subsection (const char *name)

For static functions a leading underscore is certainly okay to use,
but then ...

> +{
> +  for (unsigned i = 0; i < ARRAY_SIZE (known_subsections); ++i)
> +    if (strcmp (name, known_subsections[i].subsec_name) == 0)
> +      return &known_subsections[i];
> +  return NULL;
> +}
> +
> +static const known_tag_v2 *
> +identify_tag (const _known_subsection_v2 *subsec, uint32_t tag)

... why not be consistent one way or the other?

> +typedef unsigned char *(*display_arch_attr_t) (unsigned char *,
> +					       const unsigned char *const,
> +					       const known_tag_v2 *,
> +					       obj_attr_encoding_v2);

I won't insist on you making a change here (because I know the same style
is used elsewhere), but imo it is bad practice to hide the pointer-ness in
such a typedef, ...

> +static unsigned char *
> +display_attr_v2 (unsigned char *cursor,
> +		 const unsigned char *const end,
> +		 const char *subsec_name,
> +		 obj_attr_encoding_v2 value_encoding,
> +		 display_arch_attr_t display_arch_attr)

... making the pointer-ness entirely invisible here.

Jan


More information about the Binutils mailing list