[PATCH 03/12] libsframe: refactor code for dumping section flags

[Jens Remus]

On 29.05.2025 18:38, Indu Bhagat wrote:
> On 5/28/25 1:52 AM, Jens Remus wrote:
>> On 28.05.2025 07:40, Indu Bhagat via Binutils wrote:
>>> To prepare code for accommodating new flag additions easily as the
>>> format evolves.
>>
>>> diff --git a/libsframe/sframe-dump.c b/libsframe/sframe-dump.c

>>>   static void
>>> -dump_sframe_header (sframe_decoder_ctx *sfd_ctx)
>>> +dump_sframe_header_flags (sframe_decoder_ctx *sfd_ctx)
>>>   {
>>
>>> +  /* PS: Keep SFRAME_FLAGS_STR_MAX_LEN in sync if adding more members to
>>> +     this array.  */
>>> +#define SFRAME_FLAGS_STR_MAX_LEN 50
>>
>> I would prefer if we could get rid of this define, by using an approach
>> similar to the one found in binutils/objdump.c, dump_section_header.
>>
>>> +#define MAX_NUM_FLAGS 2
>>
>> #define MAX_NUM_FLAGS (sizeof (flags_helper) / sizeof (flags_helper[0]))

>> Using the approach from binutils/objdump.c, dump_section_header none of
>> this would be required.
>>
> 
> Hmm. OK I think something like the following may be more agreeable ?

Thanks!  Looks good to me.

> 
>   const char *prefix = "Flags: ";
>   const char *comma = ",";

Nit: Could be removed, as comma never gets re-assigned. (see below)

> 
>   flags = sframe_decoder_get_flags (sfd_ctx);
>   if (!flags)
>     {
>       printf ("%11sNONE\n", prefix);
>       return;
>     }
> 
> #define PRINT_FLAG(x, y) \
>   if (flags & x) \
>     { flags = (flags & ~x); \
>       printf ("%11s%s%s\n", prefix, y, flags ? comma : ""); \

      printf ("%11s%s%s\n", prefix, y, flags ? "," : ""); \

That would make clear, that a trailing comma or nothing gets printed and
never something else.

>       prefix = " "; \
>     }
> 
>   PRINT_FLAG (SFRAME_F_FDE_SORTED, "SFRAME_F_FDE_SORTED");
>   PRINT_FLAG (SFRAME_F_FRAME_POINTER, "SFRAME_F_FRAME_POINTER");
> #undef PRINT_FLAG


Would it make sense to print any remaining "unknown" flags in hexadecimal?

  if (flags)
    printf ("%11s%#x\n", prefix, flags);

Thanks and regards,
Jens
-- 
Jens Remus
Linux on Z Development (D3303)
+49-7031-16-1128 Office
jremus@de.ibm.com

IBM

IBM Deutschland Research & Development GmbH; Vorsitzender des Aufsichtsrats: Wolfgang Wendt; Geschäftsführung: David Faller; Sitz der Gesellschaft: Böblingen; Registergericht: Amtsgericht Stuttgart, HRB 243294
IBM Data Privacy Statement: https://www.ibm.com/privacy/