Re: [dpdk-dev] [EXT] Re: [PATCH v10 1/4] devtools: add exception for reserved fields

[Bruce Richardson <bruce.richardson@intel.com>]

On Thu, Apr 15, 2021 at 09:26:38AM +0200, David Marchand wrote:
> On Thu, Apr 15, 2021 at 7:33 AM Akhil Goyal <gakhil@marvell.com> wrote:
> >
> > Hi David,
> > > > Certain structures are added with reserved fields
> > > > to address any future enhancements to retain ABI
> > > > compatibility.
> > > > However, ABI script will still report error as it
> > > > is not aware of reserved fields. Hence, adding a
> > > > generic exception for reserved fields.
> > > >
> > > > Signed-off-by: Akhil Goyal <gakhil@marvell.com>
> > > > ---
> > > >  devtools/libabigail.abignore | 6 +++++-
> > > >  1 file changed, 5 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/devtools/libabigail.abignore b/devtools/libabigail.abignore
> > > > index 6c0b38984..654755314 100644
> > > > --- a/devtools/libabigail.abignore
> > > > +++ b/devtools/libabigail.abignore
> > > > @@ -19,4 +19,8 @@
> > > >  ; Ignore fields inserted in cacheline boundary of rte_cryptodev
> > > >  [suppress_type]
> > > >          name = rte_cryptodev
> > > > -        has_data_member_inserted_between = {offset_after(attached), end}
> > > > \ No newline at end of file
> > > > +        has_data_member_inserted_between = {offset_after(attached), end}
> > > > +
> > > > +; Ignore changes in reserved fields
> > > > +[suppress_variable]
> > > > +       name_regexp = reserved
> > > Mm, this rule is a bit scary, as it matches anything with "reserved" in it.
> >
> > Why do you feel it is scary? Reserved is something which may change at any time
> > Just like experimental. Hence creating a generic exception rule for it make sense
> > And it is done intentionally in this patch.
> 
> The reserved regexp on the name of a variable / struct field is too lax.
> Anything could be named with reserved in it.
> If we have clear patterns, they must be preferred, like (untested)
> name_regexp = ^reserved_(64|ptr)s$
> 
+1 to have a clear name. I would suggest using a "__reserved" prefix, since
no real field name should ever start with that prefix.