Re: [PATCH 1/2] app/testpmd: change rule type

[Ferruh Yigit <ferruh.yigit@amd.com>]

On 2/22/2023 2:11 PM, Eli Britstein wrote:
> Change rule type to be uintptr_t (instead of currently uint32_t) to be
> able to accomodate larger IDs, as a pre-step towards allowing user-id
> to flows.
> 

No objection to extend id storage type, but I am not clear why allowing
user-id justifies this, will user insert id more than uint32_max, or is
there some intention to partition variable for user provided ids etc?
Can you please elaborate?

> Signed-off-by: Eli Britstein <elibr@nvidia.com>
> ---
>  app/test-pmd/cmdline_flow.c | 12 ++++++------
>  app/test-pmd/config.c       | 34 ++++++++++++++++++----------------
>  app/test-pmd/testpmd.h      | 10 +++++-----
>  3 files changed, 29 insertions(+), 27 deletions(-)
> 
> diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
> index 9309607f11..a2709e8aa9 100644
> --- a/app/test-pmd/cmdline_flow.c
> +++ b/app/test-pmd/cmdline_flow.c
> @@ -1085,16 +1085,16 @@ struct buffer {
>  			uint8_t *data;
>  		} vc; /**< Validate/create arguments. */
>  		struct {
> -			uint32_t *rule;
> -			uint32_t rule_n;
> +			uintptr_t *rule;

Why not using 'uint64_t'?

As far as I know 'uintptr_t' is logically to hold pointer values, it is
good for usage like:
``
void *val;
uintptr_t ptr = (uintptr_t)val;
``

Having 'uintptr_t' as type for pointer looks confusing to me.

> +			uintptr_t rule_n;

Similarly, why not 'uint64_t' or 'size_t'?

Variable name suggest it is storing a count, if so 'size_t' is more
suitable logically (functionally I guess both are same).

Same comments apply for all below type changes.