diff mbox series

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

Message ID 20240307130742.5578-4-fengchengwen@huawei.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers
Series refine argparse library |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

fengchengwen March 7, 2024, 1:07 p.m. UTC 
  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(-)

Comments

Tyler Retzlaff March 7, 2024, 5:43 p.m. UTC | #1 
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?

>  /**
> - * Flag definition (in bitmask form) for an argument.
> + * Flag for that argument support occur multiple times.
> + * This flag can be set only when the argument is optional.
> + * When this flag is set, the callback type must be used for parsing.
>   */
> -enum rte_argparse_flag {
> -	/*
> -	 * Bits 0-1: represent the argument whether has value.
> -	 */
> -
> -	/** The argument has no value. */
> -	RTE_ARGPARSE_ARG_NO_VALUE = RTE_SHIFT_VAL64(1, 0),
> -	/** The argument must have a value. */
> -	RTE_ARGPARSE_ARG_REQUIRED_VALUE = RTE_SHIFT_VAL64(2, 0),
> -	/** The argument has optional value. */
> -	RTE_ARGPARSE_ARG_OPTIONAL_VALUE = RTE_SHIFT_VAL64(3, 0),
> -
> -
> -	/*
> -	 * Bits 2-9: represent the value type which used when autosave
> -	 */
> -
> -	/** The argument's value is int type. */
> -	RTE_ARGPARSE_ARG_VALUE_INT = RTE_SHIFT_VAL64(1, 2),
> -	/** The argument's value is uint8 type. */
> -	RTE_ARGPARSE_ARG_VALUE_U8 = RTE_SHIFT_VAL64(2, 2),
> -	/** The argument's value is uint16 type. */
> -	RTE_ARGPARSE_ARG_VALUE_U16 = RTE_SHIFT_VAL64(3, 2),
> -	/** The argument's value is uint32 type. */
> -	RTE_ARGPARSE_ARG_VALUE_U32 = RTE_SHIFT_VAL64(4, 2),
> -	/** The argument's value is uint64 type. */
> -	RTE_ARGPARSE_ARG_VALUE_U64 = RTE_SHIFT_VAL64(5, 2),
> -	/** Max value type. */
> -	RTE_ARGPARSE_ARG_VALUE_MAX = RTE_SHIFT_VAL64(6, 2),
> -
> -
> -	/**
> -	 * Bit 10: flag for that argument support occur multiple times.
> -	 * This flag can be set only when the argument is optional.
> -	 * When this flag is set, the callback type must be used for parsing.
> -	 */
> -	RTE_ARGPARSE_ARG_SUPPORT_MULTI = RTE_BIT64(10),
> -
> -	/**
> -	 * Bits 48-63: reserved for this library implementation usage.
> -	 */
> -	RTE_ARGPARSE_ARG_RESERVED_FIELD = RTE_GENMASK64(63, 48),
> -};
> +#define RTE_ARGPARSE_ARG_SUPPORT_MULTI  RTE_BIT64(10)
> +/** Reserved for this library implementation usage. */
> +#define RTE_ARGPARSE_ARG_RESERVED_FIELD RTE_GENMASK64(63, 48)
> +/**@}*/
>  
>  /**
>   * A structure used to hold argument's configuration.
> @@ -126,7 +114,7 @@ struct rte_argparse_arg {
>  	 */
>  	void *val_set;
>  
> -	/** @see rte_argparse_flag */
> +	/** Flag definition (RTE_ARGPARSE_ARG_*) for the argument. */
>  	uint64_t flags;
>  };
>  
> -- 
> 2.17.1
David Marchand March 7, 2024, 5:58 p.m. UTC | #2 
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))
Tyler Retzlaff March 7, 2024, 6:37 p.m. UTC | #3 
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
diff mbox series

Patch

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)
 /**
- * Flag definition (in bitmask form) for an argument.
+ * Flag for that argument support occur multiple times.
+ * This flag can be set only when the argument is optional.
+ * When this flag is set, the callback type must be used for parsing.
  */
-enum rte_argparse_flag {
-	/*
-	 * Bits 0-1: represent the argument whether has value.
-	 */
-
-	/** The argument has no value. */
-	RTE_ARGPARSE_ARG_NO_VALUE = RTE_SHIFT_VAL64(1, 0),
-	/** The argument must have a value. */
-	RTE_ARGPARSE_ARG_REQUIRED_VALUE = RTE_SHIFT_VAL64(2, 0),
-	/** The argument has optional value. */
-	RTE_ARGPARSE_ARG_OPTIONAL_VALUE = RTE_SHIFT_VAL64(3, 0),
-
-
-	/*
-	 * Bits 2-9: represent the value type which used when autosave
-	 */
-
-	/** The argument's value is int type. */
-	RTE_ARGPARSE_ARG_VALUE_INT = RTE_SHIFT_VAL64(1, 2),
-	/** The argument's value is uint8 type. */
-	RTE_ARGPARSE_ARG_VALUE_U8 = RTE_SHIFT_VAL64(2, 2),
-	/** The argument's value is uint16 type. */
-	RTE_ARGPARSE_ARG_VALUE_U16 = RTE_SHIFT_VAL64(3, 2),
-	/** The argument's value is uint32 type. */
-	RTE_ARGPARSE_ARG_VALUE_U32 = RTE_SHIFT_VAL64(4, 2),
-	/** The argument's value is uint64 type. */
-	RTE_ARGPARSE_ARG_VALUE_U64 = RTE_SHIFT_VAL64(5, 2),
-	/** Max value type. */
-	RTE_ARGPARSE_ARG_VALUE_MAX = RTE_SHIFT_VAL64(6, 2),
-
-
-	/**
-	 * Bit 10: flag for that argument support occur multiple times.
-	 * This flag can be set only when the argument is optional.
-	 * When this flag is set, the callback type must be used for parsing.
-	 */
-	RTE_ARGPARSE_ARG_SUPPORT_MULTI = RTE_BIT64(10),
-
-	/**
-	 * Bits 48-63: reserved for this library implementation usage.
-	 */
-	RTE_ARGPARSE_ARG_RESERVED_FIELD = RTE_GENMASK64(63, 48),
-};
+#define RTE_ARGPARSE_ARG_SUPPORT_MULTI  RTE_BIT64(10)
+/** Reserved for this library implementation usage. */
+#define RTE_ARGPARSE_ARG_RESERVED_FIELD RTE_GENMASK64(63, 48)
+/**@}*/
 
 /**
  * A structure used to hold argument's configuration.
@@ -126,7 +114,7 @@  struct rte_argparse_arg {
 	 */
 	void *val_set;
 
-	/** @see rte_argparse_flag */
+	/** Flag definition (RTE_ARGPARSE_ARG_*) for the argument. */
 	uint64_t flags;
 };