From: Arnaldo Carvalho de Melo <acme@kernel.org>
To: Douglas Raillard <douglas.raillard@arm.com>
Cc: acme@redhat.com, dwarves@vger.kernel.org
Subject: Re: [PATCH v2 3/6] fprintf: Print types only once
Date: Fri, 17 Dec 2021 15:40:36 -0300 [thread overview]
Message-ID: <YbzZpNTQ7+xgLigm@kernel.org> (raw)
In-Reply-To: <ab236940-f164-7b94-00bc-a1c685597b9a@arm.com>
Em Tue, Dec 14, 2021 at 06:23:07PM +0000, Douglas Raillard escreveu:
> On 12/14/21 2:06 PM, Arnaldo Carvalho de Melo wrote:
> > Em Tue, Dec 07, 2021 at 05:31:48PM +0000, Douglas RAILLARD escreveu:
> > > From: Douglas Raillard <douglas.raillard@arm.com>
> > >
> > > Use (struct tag).visited member to track what type has been printed
> > > already to avoid printing it twice. This allows producing a valid C
> > > header along with --expand_types.
> >
> > And here you changed the default, i.e. it is useful to have the current
> > --expand_types behaviour of expanding types as it traverse containers,
> > so I think this needs another option to ask for this new
> > behaviour, I think we can have that as --expand_types_once that would be
> > used instead of --expand_types, ok?
>
> Changed it to have --expand_types_once in the next version I'll send.
Ok, just one other nit, to make the patch smaller, please just use
continue:
diff --git a/pahole.c b/pahole.c
index f3a51cb2fe74afd2..b8beb71ae2c6a0b0 100644
--- a/pahole.c
+++ b/pahole.c
@@ -2964,6 +2964,9 @@ out_btf:
goto dump_it;
}
+ if (class && class->printed)
+ continue;
+
if (class)
class__find_holes(tag__class(class));
if (reorganize) {
> > - Arnaldo
> > > Signed-off-by: Douglas Raillard <douglas.raillard@arm.com>
> > > ---
> > > dwarves.h | 3 ++-
> > > dwarves_fprintf.c | 22 ++++++++++++++++------
> > > pahole.c | 34 ++++++++++++++++++----------------
> > > 3 files changed, 36 insertions(+), 23 deletions(-)
> > >
> > > diff --git a/dwarves.h b/dwarves.h
> > > index 52d162d..fc5b3fa 100644
> > > --- a/dwarves.h
> > > +++ b/dwarves.h
> > > @@ -413,6 +413,7 @@ struct tag {
> > > type_id_t type;
> > > uint16_t tag;
> > > bool visited;
> > > + bool printed;
> > > bool top_level;
> > > bool has_btf_type_tag;
> > > uint16_t recursivity_level;
> > > @@ -1370,7 +1371,7 @@ static inline const char *enumerator__name(const struct enumerator *enumerator)
> > > void enumeration__delete(struct type *type);
> > > void enumeration__add(struct type *type, struct enumerator *enumerator);
> > > -size_t enumeration__fprintf(const struct tag *tag_enum,
> > > +size_t enumeration__fprintf(struct tag *tag_enum,
> > > const struct conf_fprintf *conf, FILE *fp);
> > > int dwarves__init(void);
> > > diff --git a/dwarves_fprintf.c b/dwarves_fprintf.c
> > > index c5921d7..2c5dce6 100644
> > > --- a/dwarves_fprintf.c
> > > +++ b/dwarves_fprintf.c
> > > @@ -278,8 +278,8 @@ size_t typedef__fprintf(const struct tag *tag, const struct cu *cu,
> > > {
> > > struct type *type = tag__type(tag);
> > > const struct conf_fprintf *pconf = conf ?: &conf_fprintf__defaults;
> > > - const struct tag *tag_type;
> > > - const struct tag *ptr_type;
> > > + struct tag *tag_type;
> > > + struct tag *ptr_type;
> > > char bf[512];
> > > int is_pointer = 0;
> > > size_t printed;
> > > @@ -394,13 +394,14 @@ out:
> > > return type->max_tag_name_len;
> > > }
> > > -size_t enumeration__fprintf(const struct tag *tag, const struct conf_fprintf *conf, FILE *fp)
> > > +size_t enumeration__fprintf(struct tag *tag, const struct conf_fprintf *conf, FILE *fp)
> > > {
> > > struct type *type = tag__type(tag);
> > > struct enumerator *pos;
> > > int max_entry_name_len = enumeration__max_entry_name_len(type);
> > > size_t printed = fprintf(fp, "enum%s%s {\n", type__name(type) ? " " : "", type__name(type) ?: "");
> > > int indent = conf->indent;
> > > + tag->printed = 1;
> > > if (indent >= (int)sizeof(tabs))
> > > indent = sizeof(tabs) - 1;
> > > @@ -651,10 +652,14 @@ static size_t type__fprintf(struct tag *type, const struct cu *cu,
> > > size_t printed = 0;
> > > int expand_types = conf->expand_types;
> > > int suppress_offset_comment = conf->suppress_offset_comment;
> > > + bool type_printed;
> > > if (type == NULL)
> > > goto out_type_not_found;
> > > + type_printed = type->printed;
> > > + type->printed = 1;
> > > +
> > > if (conf->expand_pointers) {
> > > int nr_indirections = 0;
> > > @@ -794,7 +799,7 @@ print_default:
> > > case DW_TAG_structure_type:
> > > ctype = tag__type(type);
> > > - if (type__name(ctype) != NULL && !expand_types) {
> > > + if (type__name(ctype) != NULL && (!expand_types || type_printed)) {
> > > printed += fprintf(fp, "%s %-*s %s",
> > > (type->tag == DW_TAG_class_type &&
> > > !tconf.classes_as_structs) ? "class" : "struct",
> > > @@ -813,7 +818,7 @@ print_default:
> > > case DW_TAG_union_type:
> > > ctype = tag__type(type);
> > > - if (type__name(ctype) != NULL && !expand_types) {
> > > + if (type__name(ctype) != NULL && (!expand_types || type_printed)) {
> > > printed += fprintf(fp, "union %-*s %s", tconf.type_spacing - 6, type__name(ctype), name ?: "");
> > > } else {
> > > tconf.type_spacing -= 8;
> > > @@ -823,7 +828,7 @@ print_default:
> > > case DW_TAG_enumeration_type:
> > > ctype = tag__type(type);
> > > - if (type__name(ctype) != NULL)
> > > + if (type__name(ctype) != NULL || type_printed)
> > > printed += fprintf(fp, "enum %-*s %s", tconf.type_spacing - 5, type__name(ctype), name ?: "");
> > > else
> > > printed += enumeration__fprintf(type, &tconf, fp);
> > > @@ -1000,6 +1005,8 @@ static size_t union__fprintf(struct type *type, const struct cu *cu,
> > > uint32_t initial_union_cacheline;
> > > uint32_t cacheline = 0; /* This will only be used if this is the outermost union */
> > > + type__tag(type)->printed = 1;
> > > +
> > > if (indent >= (int)sizeof(tabs))
> > > indent = sizeof(tabs) - 1;
> > > @@ -1394,6 +1401,8 @@ static size_t __class__fprintf(struct class *class, const struct cu *cu,
> > > type__name(type) ?: "");
> > > int indent = cconf.indent;
> > > + class__tag(class)->printed = 1;
> > > +
> > > if (indent >= (int)sizeof(tabs))
> > > indent = sizeof(tabs) - 1;
> > > @@ -1856,6 +1865,7 @@ size_t tag__fprintf(struct tag *tag, const struct cu *cu,
> > > size_t printed = 0;
> > > struct conf_fprintf tconf;
> > > const struct conf_fprintf *pconf = conf;
> > > + tag->printed = 1;
> > > if (conf == NULL) {
> > > tconf = conf_fprintf__defaults;
> > > diff --git a/pahole.c b/pahole.c
> > > index f3a51cb..42ba110 100644
> > > --- a/pahole.c
> > > +++ b/pahole.c
> > > @@ -2964,22 +2964,24 @@ out_btf:
> > > goto dump_it;
> > > }
> > > - if (class)
> > > - class__find_holes(tag__class(class));
> > > - if (reorganize) {
> > > - if (class && tag__is_struct(class))
> > > - do_reorg(class, cu);
> > > - } else if (find_containers)
> > > - print_containers(cu, class_id, 0);
> > > - else if (find_pointers_in_structs)
> > > - print_structs_with_pointer_to(cu, class_id);
> > > - else if (class) {
> > > - /*
> > > - * We don't need to print it for every compile unit
> > > - * but the previous options need
> > > - */
> > > - tag__fprintf(class, cu, &conf, stdout);
> > > - putchar('\n');
> > > + if (!class->printed) {
> > > + if (class)
> > > + class__find_holes(tag__class(class));
> > > + if (reorganize) {
> > > + if (class && tag__is_struct(class))
> > > + do_reorg(class, cu);
> > > + } else if (find_containers)
> > > + print_containers(cu, class_id, 0);
> > > + else if (find_pointers_in_structs)
> > > + print_structs_with_pointer_to(cu, class_id);
> > > + else if (class) {
> > > + /*
> > > + * We don't need to print it for every compile unit
> > > + * but the previous options need
> > > + */
> > > + tag__fprintf(class, cu, &conf, stdout);
> > > + putchar('\n');
> > > + }
> > > }
> > > }
> > > --
> > > 2.25.1
> >
--
- Arnaldo
next prev parent reply other threads:[~2021-12-17 18:40 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-12-07 17:31 [PATCH v2 0/6] improve expanded header Douglas RAILLARD
2021-12-07 17:31 ` [PATCH v2 1/6] Revert "fprintf: Allow making struct/enum/union anonymous" Douglas RAILLARD
2021-12-07 17:31 ` [PATCH v2 2/6] Revert "pahole.c: Add --inner_anonymous option" Douglas RAILLARD
2021-12-07 17:31 ` [PATCH v2 3/6] fprintf: Print types only once Douglas RAILLARD
2021-12-14 14:02 ` Arnaldo Carvalho de Melo
2021-12-14 17:54 ` Douglas Raillard
2021-12-14 14:06 ` Arnaldo Carvalho de Melo
2021-12-14 18:23 ` Douglas Raillard
2021-12-17 18:40 ` Arnaldo Carvalho de Melo [this message]
2021-12-07 17:31 ` [PATCH v2 4/6] pahole.c: Add prefix to expanded type names Douglas RAILLARD
2021-12-14 14:55 ` Arnaldo Carvalho de Melo
2021-12-14 17:50 ` Douglas Raillard
2021-12-14 19:13 ` Arnaldo Carvalho de Melo
2021-12-14 19:18 ` Arnaldo Carvalho de Melo
2021-12-07 17:31 ` [PATCH v2 5/6] pahole.c: Add --expanded_prefix option Douglas RAILLARD
2021-12-07 17:31 ` [PATCH v2 6/6] pahole.c: Add --forward_decl option Douglas RAILLARD
2021-12-08 11:55 ` [PATCH v2 0/6] improve expanded header Arnaldo Carvalho de Melo
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=YbzZpNTQ7+xgLigm@kernel.org \
--to=acme@kernel.org \
--cc=acme@redhat.com \
--cc=douglas.raillard@arm.com \
--cc=dwarves@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).