Re: [PATCH v2 0/6] improve expanded header

[Arnaldo Carvalho de Melo <acme@kernel.org>]

Em Tue, Dec 07, 2021 at 05:31:45PM +0000, Douglas RAILLARD escreveu:
> From: Douglas Raillard <douglas.raillard@arm.com>
> 
> Improve headers generated with -E in a few ways:
> 
>   * Print each type only once, to avoid redefinitions
> 
>   * --forward_decl: Generate forward declaration for struct and union so
>     that uses of pointers will work.
> 
>   * --expanded_prefix: Allow prefixing names of types added by -E (all
>     the ones not explicitly requested with -C) so they can be namespaced
>     manually. This is important in order to be able to mix
>     pahole-generated headers with existing headers without clash or
>     maintenance.

Looks good from a very quick first view, I'll cut 1.23 today as the BPF
guys need a release with the new BTF tag feature and then will review
this patchset.

So I've applied so far only the reverts, since yesterday I had flipped
next to master.

- Arnaldo
 
> 
> types.txt:
> 
>   t1
>   t4
> 
> Original printed type with "-C types.txt -E":
> 
>   struct t1 {
>     struct t2 *a;
>     struct t3 {
>       int a;
>     } b;
>   };
> 
>   struct t4 {
>     struct t3 {
>       int a;
>     } a;
>   };
> 
> After this series and with
> "-C types.txt -E --expanded_prefix __pahole --forward_decl":
> 
>   struct t1
>   struct t2;
>   struct t3;
> 
>   struct t1 {
>     struct t2 *a;
>     struct __pahole_t3 {
>       int a;
>     } b;
>   };
> 
>   struct t4 {
>     struct __pahole_t3 a;
>   };
> 
> 
> This header can be freely mixed with any other header as long as they
> don't define t1 or t4 (if there is a clash, then pahole is not necessary
> and the user can just use the header in the first place).
> 
> TODOs:
> 
>   * Define types on their first use even when they are first used as
>     pointers. Currently, if a type is only used as pointer it will never
>     get defined, which will still compile thanks to --forward_decl but
>     won't allow easy use of the values.
> 
>   * The prefix system allocates memory to rename types and never frees
>     it. I don't think it's a big issue for now as it's done inside
>     pahole.c and type names are likely necessary until the end of the
>     process anyway, but it makes valgrind angry and could therefore mask
>     other problems. There is another implementation of prefix that
>     modifies dwarves_fprintf.c instead of changing the name but it was
>     very invasive and impossible to check if prefixable spots were
>     missed.
> 
> 
> This series is to be applied on the "next" branch as it includes reverts
> of the --inner_anonymous series, which is dropped since it could not
> cope with recursive types.
> 
> 
> Douglas Raillard (6):
>   Revert "fprintf: Allow making struct/enum/union anonymous"
>   Revert "pahole.c: Add --inner_anonymous option"
>   fprintf: Print types only once
>   pahole.c: Add prefix to expanded type names
>   pahole.c: Add --expanded_prefix option
>   pahole.c: Add --forward_decl option
> 
>  dwarves.h          | 12 ++++--
>  dwarves_emit.c     |  2 +-
>  dwarves_fprintf.c  | 77 +++++++++++++++++-------------------
>  man-pages/pahole.1 |  8 ++--
>  pahole.c           | 98 ++++++++++++++++++++++++++++++++++------------
>  5 files changed, 121 insertions(+), 76 deletions(-)
> 
> -- 
> 2.25.1

-- 

- Arnaldo