Re: [PATCH 2/6] app/testpmd: register driver specific commands

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

On 5/24/2022 11:53 AM, David Marchand wrote:
> [CAUTION: External Email]
> 
> On Mon, May 23, 2022 at 8:10 PM Ferruh Yigit <ferruh.yigit@amd.com> wrote:
>>
>> On 5/23/2022 8:10 AM, David Marchand wrote:
>>> +int
>>> +init_cmdline(void)
>>> +{
>>> +       struct testpmd_commands *c;
>>> +       cmdline_parse_ctx_t *ctx;
>>> +       unsigned int count = 0;
>>> +       unsigned int i;
>>> +
>>> +       /* initialize non-constant commands */
>>> +       cmd_set_fwd_mode_init();
>>> +       cmd_set_fwd_retry_mode_init();
>>> +
>>> +       main_ctx = NULL;
>>> +       for (i = 0; builtin_ctx[i] != NULL; i++) {
>>> +               ctx = realloc(main_ctx, (count + i + 1) * sizeof(*ctx));
>>> +               if (ctx == NULL)
>>> +                       goto err;
>>> +               main_ctx = ctx;
>>
>> Instead of 'realloc()', check and assign pointer in each entry, what do
>> you think about first calculate the size and alloc once, both for
>> 'builtin_ctx' & driver specific command?
>> But of course trade off is to loop twice in that case.
> 
> Yes, I hesitated because of the duplicated loop (and I found no
> "elegant" macro).
> I'm ok with the suggestion, this makes the code simpler.
> 
> 
> int
> init_cmdline(void)
> {
>          struct testpmd_commands *c;
>          unsigned int count;
>          unsigned int i;
> 
>          /* initialize non-constant commands */
>          cmd_set_fwd_mode_init();
>          cmd_set_fwd_retry_mode_init();
> 
>          count = 0;
>          for (i = 0; builtin_ctx[i] != NULL; i++, count++)
>                  ;
>          TAILQ_FOREACH(c, &commands_head, next) {
>                  for (i = 0; c->commands[i].ctx != NULL; i++, count++)
>                          ;
>          }
> 
>          /* cmdline expects a NULL terminated array */
>          main_ctx = calloc(count + 1, sizeof(main_ctx[0]));
>          if (main_ctx == NULL)
>                  return -1;
> 
>          count = 0;
>          for (i = 0; builtin_ctx[i] != NULL; i++, count++)
>                  main_ctx[count] = builtin_ctx[i];
>          TAILQ_FOREACH(c, &commands_head, next) {
>                  for (i = 0; c->commands[i].ctx != NULL; i++, count++)
>                          main_ctx[count] = c->commands[i].ctx;
>          }
> 
>          return 0;
> }
> 

ack