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

[Indu Bhagat]

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
> 
>> +struct dump_flags_helper
>> +{
>> +  uint8_t flag;
>> +  const char *flag_str;
>> +};
>> +
> 
> Couldn't this be an anonymous struct when defining flags_helper in
> dump_sframe_header_flags? See below.
> 
>>   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]))
> 
> Or see below.
> 
>> +  const struct dump_flags_helper flags_helper[MAX_NUM_FLAGS] = {
>> +      { SFRAME_F_FDE_SORTED, "SFRAME_F_FDE_SORTED"},
>> +      { SFRAME_F_FRAME_POINTER, "SFRAME_F_FRAME_POINTER"}
>> +  };
> 
> const struct {
>    uint8_t flag;
>    const char *string;
> } flag_strings[] = {
>    { SFRAME_F_FDE_SORTED, "SFRAME_F_FDE_SORTED"},
>    { SFRAME_F_FRAME_POINTER, "SFRAME_F_FRAME_POINTER"},
> };
> 
> const size_t num_flags = sizeof (flag_strings) / sizeof (flag_strings[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 ?

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

   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 : ""); \
       prefix = " "; \
     }

   PRINT_FLAG (SFRAME_F_FDE_SORTED, "SFRAME_F_FDE_SORTED");
   PRINT_FLAG (SFRAME_F_FRAME_POINTER, "SFRAME_F_FRAME_POINTER");
#undef PRINT_FLAG

Thanks for reviewing