[U-Boot] [PATCH v2 1/2] common: command: Use command_ret_t enum values instead of values

[Simon Glass]

Hi Michal,

On 22 June 2018 at 00:33, Michal Simek <michal.simek at xilinx.com> wrote:
>
> On 21.6.2018 21:45, Simon Glass wrote:
> > On 21 June 2018 at 06:58, Michal Simek <michal.simek at xilinx.com> wrote:
> >> Use enum command_ret_t types in cmd_process_error().
> >>
> >> Signed-off-by: Michal Simek <michal.simek at xilinx.com>
> >> ---
> >>
> >> Changes in v2:
> >> - Move adding RET_USAGE to separate patch.
> >>
> >>  common/command.c | 4 ++--
> >>  1 file changed, 2 insertions(+), 2 deletions(-)
> >>
> >
> > Reviewed-by: Simon Glass <sjg at chromum.org>
> >
> >
> >> diff --git a/common/command.c b/common/command.c
> >> index 52d47c133c3c..a4a8dc601acb 100644
> >> --- a/common/command.c
> >> +++ b/common/command.c
> >> @@ -549,8 +549,8 @@ int cmd_process_error(cmd_tbl_t *cmdtp, int err)
> >>  {
> >>         if (err) {
> >>                 printf("Command '%s' failed: Error %d\n", cmdtp->name, err);
> >> -               return 1;
> >> +               return CMD_RET_FAILURE;
> >>         }
> >>
> >> -       return 0;
> >> +       return CMD_RET_SUCCESS;
> >
> > I actually thing 0 is fine here. That is the definition of success.
>
> and CMD_RET_SUCCESS has this 0 value too.
>
> maybe would be worth to also change return type to enum command_ret_t
> as is done for cmd_process.
>
> For example ubi_remove_vol() in case of failure returs +ENODEV and others.
> I thought that commands could return only 3 values convert by enum
> command_ret_t. Or is it ok to return also different values?

Commands should only return things from the enum. My point was that I
find 'return CMD_RET_SUCCESS' to be a bit painful. We know the value
is 0 and it is much shorter to read, so I prefer 'return 0' instead of
'return CMD_RET_SUCCESS'

Also I like this:

if (!xx)
   // we got an error

and don't like this:

if (xx != CMD_RET_SUCCESS)
   // we got an error

Regards,
Simon