Dear Mike,
> On Tuesday 03 July 2012 05:38:09 Lukasz Majewski wrote:
> > --- /dev/null
> > +++ b/common/cmd_dfu.c
> >
> > +int do_dfu(cmd_tbl_t *cmdtp, int flag, int argc, char * const
> > argv[])
>
> static
It can be static (static int do_dfu). On the other hand the U_BOOT_CMD
macro defines:
int (*cmd)(struct cmd_tbl_s *, int, int, char * const []); at
struct cmd_tbl_s, which is int.
>
> > +{
> > + char *str_env = NULL, *env_bkp = NULL;
>
> no need to assign NULL here. str_env should be const.
Yes, correct.
>
> > + static char *s = "dfu";
>
> no need to declare this static
Why not? The s ptr is further passed to g_dnl_init(s), so my intend was
to declare string in the data segment of this translation unit. In this
way the data under this ptr will not vanish.
>
> > + int ret = 0;
>
> no need to init to 0
Ok.
>
> > + env_bkp = strdup(str_env);
> > + ret = dfu_config_entities(env_bkp, argv[1],
> > + (int)simple_strtoul(argv[2], NULL,
> > 10));
> > + if (ret)
> > + return CMD_RET_FAILURE;
> > +
> > + if (strcmp(argv[3], "list") == 0) {
> > + dfu_show_entities();
> > + dfu_free_entities();
> > + free(env_bkp);
> > + return CMD_RET_SUCCESS;
>
> for these last three statements, you could just do "goto done" and
> put a done label below ...
Yes, you are correct. Thanks for tip.
>
> > +exit:
> > + g_dnl_cleanup();
>
> done:
>
> > + dfu_free_entities();
> > + free(env_bkp);
> > +
> > + return CMD_RET_SUCCESS;
>
> -mike
--
Best regards,
Lukasz Majewski
Samsung Poland R&D Center | Linux Platform Group
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
