| Message ID | 272bf4ea65f505eec557648ba5149fcaf4a63b91.1528112260.git.michal.simek@xilinx.com |
|---|---|
| State | Deferred |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | [U-Boot] common: Use command_ret_t enum values instead of values | expand |
On 06/04/2018 01:37 PM, Michal Simek wrote: > Use enum command_ret_t types in cmd_process_error(). > Also handle USAGE failure separately. > > Signed-off-by: Michal Simek <michal.simek@xilinx.com> > --- > > common/command.c | 7 +++++-- > 1 file changed, 5 insertions(+), 2 deletions(-) > > diff --git a/common/command.c b/common/command.c > index 52d47c133c3c..2433a89e0a8e 100644 > --- a/common/command.c > +++ b/common/command.c > @@ -547,10 +547,13 @@ enum command_ret_t cmd_process(int flag, int argc, char * const argv[], > > int cmd_process_error(cmd_tbl_t *cmdtp, int err) > { > + if (err == CMD_RET_USAGE) > + return CMD_RET_USAGE; This changes the return value of the function (-1 instead of 1). So you would have to change the comments in include/command.h. Your commit message gives no clue why you do not want to return CMD_RET_FAILURE here and which consequences this has especially in scripts. Best regards Heinrich > + > if (err) { > printf("Command '%s' failed: Error %d\n", cmdtp->name, err); > - return 1; > + return CMD_RET_FAILURE; > } > > - return 0; > + return CMD_RET_SUCCESS; > } >
On 4.6.2018 21:22, Heinrich Schuchardt wrote: > On 06/04/2018 01:37 PM, Michal Simek wrote: >> Use enum command_ret_t types in cmd_process_error(). >> Also handle USAGE failure separately. >> >> Signed-off-by: Michal Simek <michal.simek@xilinx.com> >> --- >> >> common/command.c | 7 +++++-- >> 1 file changed, 5 insertions(+), 2 deletions(-) >> >> diff --git a/common/command.c b/common/command.c >> index 52d47c133c3c..2433a89e0a8e 100644 >> --- a/common/command.c >> +++ b/common/command.c >> @@ -547,10 +547,13 @@ enum command_ret_t cmd_process(int flag, int argc, char * const argv[], >> >> int cmd_process_error(cmd_tbl_t *cmdtp, int err) >> { >> + if (err == CMD_RET_USAGE) >> + return CMD_RET_USAGE; > > This changes the return value of the function (-1 instead of 1). So you > would have to change the comments in include/command.h. thanks for pointer. > > Your commit message gives no clue why you do not want to return > CMD_RET_FAILURE here and which consequences this has especially in scripts. I will improve commit message in v2. CMD_RET_USAGE is one of 3 return values which should be handled. These commands are affected by this change. cmd/demo.c cmd/efi.c cmd/gpio.c cmd/qfw.c cmd/x86/fsp.c test/dm/cmd_dm.c And scripts shouldn't be affected because return value is not 0. But every command implementation can choose what it is correct to pass. I would expect that RET_USAGE is called when parameters are not correctly passed (have incorrect value, missing parameters) and RET_FAILURE when correct parameters are passed but command fails. Thanks, Michal
diff --git a/common/command.c b/common/command.c index 52d47c133c3c..2433a89e0a8e 100644 --- a/common/command.c +++ b/common/command.c @@ -547,10 +547,13 @@ enum command_ret_t cmd_process(int flag, int argc, char * const argv[], int cmd_process_error(cmd_tbl_t *cmdtp, int err) { + if (err == CMD_RET_USAGE) + return CMD_RET_USAGE; + if (err) { printf("Command '%s' failed: Error %d\n", cmdtp->name, err); - return 1; + return CMD_RET_FAILURE; } - return 0; + return CMD_RET_SUCCESS; }
Use enum command_ret_t types in cmd_process_error(). Also handle USAGE failure separately. Signed-off-by: Michal Simek <michal.simek@xilinx.com> --- common/command.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)