Re: [PATCH v3 2/7] drivers/bus: eliminate dependency on non-portable __SIZEOF_LONG__

[Thomas Monjalon <thomas@monjalon.net>]

06/12/2024 19:14, Andre Muezerie:
> On Fri, Dec 06, 2024 at 04:41:16PM +0000, Konstantin Ananyev wrote:
> > 
> > > > > Macro __SIZEOF_LONG__ is not standardized and MSVC does not define it.
> > > > > Therefore the errors below are seen with MSVC:
> > > > >
> > > > > ../lib/mldev/mldev_utils_scalar.c(465): error C2065:
> > > > >     '__SIZEOF_LONG__': undeclared identifier
> > > > > ../lib/mldev/mldev_utils_scalar.c(478): error C2051:
> > > > >     case expression not constant
> > > > >
> > > > > ../lib/mldev/mldev_utils_scalar_bfloat16.c(33): error C2065:
> > > > >     '__SIZEOF_LONG__': undeclared identifier
> > > > > ../lib/mldev/mldev_utils_scalar_bfloat16.c(49): error C2051:
> > > > >     case expression not constant
> > > > >
> > > > > Turns out that the places where __SIZEOF_LONG__ is currently
> > > > > being used can equally well use sizeof(long) instead.
> > > > >
> > > > > Signed-off-by: Andre Muezerie <andremue@linux.microsoft.com>
> > > > > ---
> > > > >  drivers/bus/fslmc/mc/fsl_mc_cmd.h | 3 +--
> > > > >  1 file changed, 1 insertion(+), 2 deletions(-)
> > > > >
> > > > > diff --git a/drivers/bus/fslmc/mc/fsl_mc_cmd.h b/drivers/bus/fslmc/mc/fsl_mc_cmd.h
> > > > > index a768774c89..f27a18905d 100644
> > > > > --- a/drivers/bus/fslmc/mc/fsl_mc_cmd.h
> > > > > +++ b/drivers/bus/fslmc/mc/fsl_mc_cmd.h
> > > > > @@ -29,9 +29,8 @@
> > > > >  #define le32_to_cpu	rte_le_to_cpu_32
> > > > >  #define le16_to_cpu	rte_le_to_cpu_16
> > > > >
> > > > > -#define BITS_PER_LONG	(__SIZEOF_LONG__ * 8)
> > > > >  #define GENMASK(h, l) \
> > > > > -		(((~0UL) << (l)) & (~0UL >> (BITS_PER_LONG - 1 - (h))))
> > > > > +		(((~0UL) << (l)) & (~0UL >> (RTE_BITS_PER_LONG - 1 - (h))))
> > > >
> > > > Inside
> > > > There are macros: RTE_GENMASK64 (and RTE_GENMASK32).
> > > > Which as I understand does same thing.
> > > > Might be better to just use these ones instead of hand-crafting
> > > > same thing over different PMDs.
> > > >
> > > 
> > > I agree. I can submit a separate patch for that once this series aimed at
> > > unblocking MSVC from being used on this code goes in, if that is OK. If
> > > instead you feel strongly that this change should be made as part of this
> > > series let me know.
> > 
> > I am fine anyway.
> > Just thought that doing something like:
> > #define GENMASK(h, l)	RTE_GENMASK64(h, l)
> > Would be less work for you and less code churn.
> > 
> 
> I thought about that option too, but getting rid of that "redefine" and just 
> replace all occurrences of GENMASK would be better I think, despite
> the code churn.

Yes please, it is better to remove these macros and directly use RTE_ equivalents where needed.
Thanks.