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

[Jan Beulich]

On 18.08.2025 17:18, Matthieu Longo wrote:
> 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.

I didn't mean to ask that you change that label's placement. I merely
used it to indicate what happens if you add labels (elsewhere) without
any indentation.

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

Try making a local variable of this struct type without initializer, and
then set all the fields using ordinary statements.

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

Did you read my remark as suggestion to drop the typedef? I'm fine with
that. What I would prefer is if it was like this:

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

and hence

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)

(pointer-ness now visible).

Jan