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

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

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;
}


-- 
David Marchand