RE: [EXTERNAL] [PATCH v3 4/6] trace: support dumping binary inside a struct

[Sunil Kumar Kori <skori@marvell.com>]

> On Tue, Feb 11, 2025 at 9:53 AM Sunil Kumar Kori <skori@marvell.com>
> wrote:
> >
> > > diff --git a/lib/eal/common/eal_common_trace_ctf.c
> > > b/lib/eal/common/eal_common_trace_ctf.c
> > > index 6bc8bb9036..d9b307e076 100644
> > > --- a/lib/eal/common/eal_common_trace_ctf.c
> > > +++ b/lib/eal/common/eal_common_trace_ctf.c
> > > @@ -378,6 +378,9 @@ char *trace_metadata_fixup_field(const char
> *field)
> > >               "->",
> > >               "*",
> > >               " ",
> > > +             "&",
> > > +             "(",
> > > +             ")",
> > Adding brackets makes token names a bit complex. Same name is used in
> > metadata file to dump the traces to the user. With this complex name,
> > user might not understand the purpose of that information.
> >
> > For example, _conf_src_port_pcie_sizeof_uint64_t_ is created in
> > metafile and same will be dumped. But with this User might not get that
> which information is provided.
> 
> In practice, as there is no other documentation for a trace point arguments, a
> user needs to read the trace point definitions.
> So it seems trivial to me to link a variable name in the trace point emitter, and
> the metadata in the trace files.
> 
> >
> > This is the reason; we followed the existing naming convention which is user
> friendly.
> 
> User friendly? I don't see how this is different with '.' and '->'.
In general, structure fields are given a proper name to represent the purpose.
When we use it directly in trace point using '.' or '->' then it remains a meaningful name.
Adding more tokens in name, is making them complex and deviating from there meaning. 

I am not saying that the mentioned support should not be there. I am just trying to convey
that If it is possible to make meaningful names, then that will be more helpful. 

> What is missing is documentation, as nothing describes this sanitisation in the
> first place (even before this series).
> 
> 
> --
> David Marchand