Re: [PATCH v2 3/5] argparse: replace flag enum with marco

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

On Thu, Mar 07, 2024 at 06:58:20PM +0100, David Marchand wrote:
> On Thu, Mar 7, 2024 at 6:43 PM Tyler Retzlaff
> <roretzla@linux.microsoft.com> wrote:
> >
> > On Thu, Mar 07, 2024 at 01:07:40PM +0000, Chengwen Feng wrote:
> > > The enum rte_argparse_flag's value is u64, but an enum in C is
> > > represented as an int. This commit replace these enum values with
> > > macro.
> > >
> > > Fixes: e3e579f5bab5 ("argparse: introduce argparse library")
> > > Fixes: 5357c248c960 ("argparse: parse unsigned integers")
> > >
> > > Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
> > > ---
> > >  lib/argparse/rte_argparse.h | 78 ++++++++++++++++---------------------
> > >  1 file changed, 33 insertions(+), 45 deletions(-)
> > >
> > > diff --git a/lib/argparse/rte_argparse.h b/lib/argparse/rte_argparse.h
> > > index 47e231bef9..a6a7790cb4 100644
> > > --- a/lib/argparse/rte_argparse.h
> > > +++ b/lib/argparse/rte_argparse.h
> > > @@ -37,52 +37,40 @@
> > >  extern "C" {
> > >  #endif
> > >
> > > +/**@{@name Flag definition (in bitmask form) for an argument
> > > + *
> > > + * @note Bits[0~1] represent the argument whether has value,
> > > + * bits[2~9] represent the value type which used when autosave.
> > > + *
> > > + * @see struct rte_argparse_arg::flags
> > > + */
> > > +/** The argument has no value. */
> > > +#define RTE_ARGPARSE_ARG_NO_VALUE       RTE_SHIFT_VAL64(1, 0)
> > > +/** The argument must have a value. */
> > > +#define RTE_ARGPARSE_ARG_REQUIRED_VALUE RTE_SHIFT_VAL64(2, 0)
> > > +/** The argument has optional value. */
> > > +#define RTE_ARGPARSE_ARG_OPTIONAL_VALUE RTE_SHIFT_VAL64(3, 0)
> > > +/** The argument's value is int type. */
> > > +#define RTE_ARGPARSE_ARG_VALUE_INT      RTE_SHIFT_VAL64(1, 2)
> > > +/** The argument's value is uint8 type. */
> > > +#define RTE_ARGPARSE_ARG_VALUE_U8       RTE_SHIFT_VAL64(2, 2)
> > > +/** The argument's value is uint16 type. */
> > > +#define RTE_ARGPARSE_ARG_VALUE_U16      RTE_SHIFT_VAL64(3, 2)
> > > +/** The argument's value is uint32 type. */
> > > +#define RTE_ARGPARSE_ARG_VALUE_U32      RTE_SHIFT_VAL64(4, 2)
> > > +/** The argument's value is uint64 type. */
> > > +#define RTE_ARGPARSE_ARG_VALUE_U64      RTE_SHIFT_VAL64(5, 2)
> > > +/** Max value type. */
> > > +#define RTE_ARGPARSE_ARG_VALUE_MAX      RTE_SHIFT_VAL64(6, 2)
> >
> > it was good to get rid of the enum but will the above macros expand to
> > signed or unsigned integer given the type of field could we make sure
> > they expand unsigned?
> 
> Afaiu, those should expand to unsigned:
> #define RTE_SHIFT_VAL64(val, nr) (UINT64_C(val) << (nr))

Thanks for confirming!

> 
> 
> -- 
> David Marchand