Re: [RFC PATCH v2 1/3] eal/windows: move fnmatch function to header file

[Tyler Retzlaff <roretzla@linux.microsoft.com>]

On Fri, Jan 13, 2023 at 05:01:20PM +0000, Bruce Richardson wrote:
> On Fri, Jan 13, 2023 at 05:41:29PM +0100, Morten Brørup wrote:
> > > From: Bruce Richardson [mailto:bruce.richardson@intel.com]
> > > Sent: Friday, 13 January 2023 17.20
> > > 
> > > To allow the fnmatch function to be shared between libraries, without
> > > having to export it into the public namespace (since it's not prefixed
> > > with "rte"), we can convert fnmatch.c to replace fnmatch.h. This allows
> > > fnmatch function to be static and limited in scope to the current file,
> > > preventing duplicate definitions if it is used by two libraries, while
> > > also not requiring export for sharing.
> > > 
> > > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> > > ---
> > 
> > [...]
> > 
> > >  #define FNM_CASEFOLD 0x10
> > >  #define FNM_PREFIX_DIRS 0x20
> > > 
> > > +#define EOS	'\0'
> > 
> > Careful about names in header files. Perhaps EOS should also have the FNM_ name space prefix to reduce the risk of collision. Or even better: just use '\0' in the code instead of defining a special name for it.
> > 
> > > +
> > > +static const char *rangematch(const char *, char, int);
> > 
> > I don't think rangematch() is a POSIX function, so similar comment here. Prefix with fnm_ to reduce risk of collision.
> > 
> > With those fixes...
> > 
> > Series-acked-by: Morten Brørup <mb@smartsharesystems.com>
> > 
> 
> Sure, I can prefix those.
> 
> However, since this is a non-published header, private to the DPDK build,
> the only chances of collision come from the DPDK files which include the
> header. The objective of making it a header was so that none of this ever
> leaks outside of the DPDK libs that use the functions internally. That
> said, there is no harm in prefixing either, so I'll do so in any future
> version.

and if a future collision occurs we should be able to adapt without
compat break. you know for platforms that step on the application
namespace... *cough* legacy windows headers.

thank you for doing this Bruce.

Series-acked-by: Tyler Retzlaff <roretzla@linux.microsoft.com>