Hi Michal, On 22 June 2018 at 00:33, Michal Simek <[email protected]> wrote: > > On 21.6.2018 21:45, Simon Glass wrote: > > On 21 June 2018 at 06:58, Michal Simek <[email protected]> wrote: > >> Use enum command_ret_t types in cmd_process_error(). > >> > >> Signed-off-by: Michal Simek <[email protected]> > >> --- > >> > >> 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 <[email protected]> > > > > > >> 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 _______________________________________________ U-Boot mailing list [email protected] https://lists.denx.de/listinfo/u-boot
