Re: [dpdk-dev] [PATCH v2 1/3] app: use RTE_DIM to calculate array size

[David Marchand <david.marchand@redhat.com>]

On Thu, Nov 7, 2019 at 3:59 AM <pbhagavatula@marvell.com> wrote:
>
> From: Pavan Nikhilesh <pbhagavatula@marvell.com>
>
> Use RTE_DIM macro to calculate array size
>
> Suggested-by: David Marchand <david.marchand@redhat.com>
> Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
> ---
>  v2 Changes:
>  ----------
>  - remove macros that are used only once.(David)
>
>  app/test-pmd/cmdline.c            |  2 +-
>  app/test-pmd/icmpecho.c           |  2 +-
>  app/test-pmd/testpmd.c            |  2 --
>  app/test/test.c                   |  2 +-
>  app/test/test_cmdline_etheraddr.c | 15 +++------------
>  app/test/test_cmdline_ipaddr.c    | 25 ++++++-------------------
>  app/test/test_cmdline_num.c       | 23 +++++------------------
>  app/test/test_cmdline_portlist.c  | 17 ++++-------------
>  app/test/test_cmdline_string.c    | 23 +++++------------------
>  app/test/test_debug.c             |  2 +-
>  app/test/test_eal_flags.c         |  9 ++++-----
>  app/test/test_errno.c             |  4 ++--
>  app/test/test_lpm.c               |  3 +--
>  app/test/test_lpm6.c              |  3 +--
>  app/test/test_lpm6_data.h         |  3 +--
>  app/test/test_malloc.c            |  2 +-
>  app/test/test_memcpy.c            |  2 +-
>  app/test/test_memcpy_perf.c       |  4 ++--
>  app/test/test_mp_secondary.c      |  3 +--
>  app/test/test_pdump.c             |  3 +--
>  app/test/test_pmd_ring_perf.c     |  2 +-
>  app/test/test_ring_perf.c         |  6 +++---
>  app/test/test_timer_secondary.c   |  3 +--
>  23 files changed, 47 insertions(+), 113 deletions(-)
>
> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
> index 447806991..c6b4e44a2 100644
> --- a/app/test-pmd/cmdline.c
> +++ b/app/test-pmd/cmdline.c
> @@ -5490,7 +5490,7 @@ cmd_show_bypass_config_parsed(void *parsed_result,
>                 "OS/board off",
>                 "power supply off",
>                 "timeout"};
> -       int num_events = (sizeof events) / (sizeof events[0]);
> +       int num_events = RTE_DIM(events);

num_events is used only once.

>
>         /* Display the bypass mode.*/
>         if (rte_pmd_ixgbe_bypass_state_show(port_id, &bypass_mode) != 0) {

[snip]

> diff --git a/app/test/test_cmdline_num.c b/app/test/test_cmdline_num.c
> index 4c97caf3d..f6af58a27 100644
> --- a/app/test/test_cmdline_num.c
> +++ b/app/test/test_cmdline_num.c
[snip]
> @@ -431,7 +418,7 @@ test_parse_num_valid(void)
>                 token.num_data.type = type;
>
>                 /* test positive strings */
> -               for (i = 0; i < NUM_POSITIVE_STRS_SIZE; i++) {
> +               for (i = 0; i <  RTE_DIM(num_valid_positive_strs); i++) {

Unneeded space.

>                         result = 0;
>                         memset(&buf, 0, sizeof(buf));
>

[snip]

> diff --git a/app/test/test_cmdline_portlist.c b/app/test/test_cmdline_portlist.c
> index 0dc6d0030..cbfdfcf54 100644
> --- a/app/test/test_cmdline_portlist.c
> +++ b/app/test/test_cmdline_portlist.c
> @@ -6,6 +6,7 @@
>  #include <string.h>
>  #include <inttypes.h>
>
> +#include <rte_common.h>

We separate the includes in 3 groups, standard system headers, dpdk
headers and finally application headers.
Add a line here.

>  #include <cmdline_parse.h>
>  #include <cmdline_parse_portlist.h>
>

[snip]

> diff --git a/app/test/test_memcpy.c b/app/test/test_memcpy.c
> index 2c69ad964..53eb8433a 100644
> --- a/app/test/test_memcpy.c
> +++ b/app/test/test_memcpy.c
> @@ -103,7 +103,7 @@ static int
>  func_test(void)
>  {
>         unsigned int off_src, off_dst, i;
> -       unsigned int num_buf_sizes = sizeof(buf_sizes) / sizeof(buf_sizes[0]);
> +       unsigned int num_buf_sizes = RTE_DIM(buf_sizes);

num_buf_sizes is used only once.

>         int ret;
>
>         for (off_src = 0; off_src < ALIGNMENT_UNIT; off_src++) {
> diff --git a/app/test/test_memcpy_perf.c b/app/test/test_memcpy_perf.c
> index 6f436f3ef..8f06b0f1e 100644
> --- a/app/test/test_memcpy_perf.c
> +++ b/app/test/test_memcpy_perf.c
> @@ -250,7 +250,7 @@ perf_test_constant_unaligned(void)
>  static inline void
>  perf_test_variable_aligned(void)
>  {
> -       unsigned n = sizeof(buf_sizes) / sizeof(buf_sizes[0]);
> +       unsigned n = RTE_DIM(buf_sizes);

n is used only once.

>         unsigned i;
>         for (i = 0; i < n; i++) {
>                 ALL_PERF_TESTS_FOR_SIZE((size_t)buf_sizes[i]);
> @@ -261,7 +261,7 @@ perf_test_variable_aligned(void)
>  static inline void
>  perf_test_variable_unaligned(void)
>  {
> -       unsigned n = sizeof(buf_sizes) / sizeof(buf_sizes[0]);
> +       unsigned n = RTE_DIM(buf_sizes);

idem.

>         unsigned i;
>         for (i = 0; i < n; i++) {
>                 ALL_PERF_TESTS_FOR_SIZE_UNALIGNED((size_t)buf_sizes[i]);

The rest looks good.

If you still have some available cycles, can you handle those too?

$ git grep 'sizeof(.*) */ *sizeof(.*)' -- app/test
app/test/test_func_reentrancy.c:   for (case_id = 0; case_id <
sizeof(test_cases)/sizeof(struct test_case); case_id ++) {
app/test/test_hash.c:           i < sizeof(hashtest_funcs) /
sizeof(rte_hash_function);
app/test/test_hash.c:                   j < sizeof(hashtest_initvals)
/ sizeof(uint32_t);
app/test/test_hash.c:                           k <
sizeof(hashtest_key_lens) / sizeof(uint32_t);
app/test/test_hash_functions.c:    for (i = 0; i <
sizeof(hashtest_key_lens) / sizeof(uint32_t); i++) {
app/test/test_hash_functions.c:            for (j = 0; j <
sizeof(hashtest_initvals) / sizeof(uint32_t); j++) {
app/test/test_hash_functions.c:    for (i = 0; i <
sizeof(hashtest_key_lens) / sizeof(uint32_t); i++) {
app/test/test_hash_functions.c:            for (j = 0; j <
sizeof(hashtest_initvals) / sizeof(uint32_t); j++) {
app/test/test_power.c:     const int envs_size =
sizeof(envs)/sizeof(enum power_management_env);


Thanks for working on cleanups.


--
David Marchand