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

[Matthieu Longo]

On 2025-08-08 13:42, Jan Beulich wrote:
> 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.
> 

This is not part of my changes but I fixed it in the next revision.
I don't think that it is worth splitting it in its own patch.

>>     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".
> 

I removed the "const" in the next revision.
Why would you recommend against ?

>> +} _known_subsection_v2;
> 
> Any particular need for the leading underscore here?
> 

Fixed in the next revision.

>> +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.
> 

Fixed in the next revision.

>> +static const _known_subsection_v2 *
>> +_identify_subsection (const char *name)
> 
> For static functions a leading underscore is certainly okay to use,
> but then ...
> 

Fixed in the next revision.

>> +{
>> +  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?
> 

I think there was a conflict previously, hence the leading _. Since the 
conflict does not exist anymore, no need to keep it.

>> +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.

The signature is so long that it becomes unreadable if you keep the 
original signature in the function parameters.
I understand your concern, but in this specific case, I rather think 
that it is useful and is worth the cost of hiding the pointer. Modern 
editors don't have issue displaying the detailed type easily. No idea is 
this is less easy with editors like vim or emacs.

Matthieu

> 
> Jan