[PATCH, V2 2/9] libsframe: refactor code for dumping section flags

[Jan Beulich]

On 04.06.2025 09:08, Indu Bhagat via Binutils wrote:
> @@ -40,12 +38,35 @@ is_sframe_abi_arch_aarch64 (sframe_decoder_ctx *sfd_ctx)
>    return aarch64_p;
>  }
>  
> +static void
> +dump_sframe_header_flags (sframe_decoder_ctx *sfd_ctx)

It's really a shame that this can't be pointer-to-const. Dumping functions
better would make quite clear that they're not altering what is being dumped.

> +{
> +  uint8_t flags;
> +  const char *prefix = "Flags: ";
> +
> +  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); \

To play safe, please parenthesize the uses of x in expressions. Otoh ...

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

... this use of y doesn't need parentheses, and adding ones would only
hamper readability.

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

This way the macro doesn't really need two parameters, does it? You
could simply use #x there in place of y. And then actually

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

As to the printing of the comma: If further flags are set that the
implementation doesn't know of, you will end up printing a comma with
nothing following. That's odd. Perhaps print the residual at the end?

Jan