[PATCH 10/15] cmd: blk_common: Use macros for the return values

[Simon Glass]

Hi Bin,

On Tue, 26 Sept 2023 at 02:54, Bin Meng <bmeng at tinylab.org> wrote:
>
> Avoid using magic number 0/1 for the command result.
>
> Signed-off-by: Bin Meng <bmeng at tinylab.org>
> ---
>
>  cmd/blk_common.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/cmd/blk_common.c b/cmd/blk_common.c
> index 9f9d4327a9..ad9b16dc09 100644
> --- a/cmd/blk_common.c
> +++ b/cmd/blk_common.c
> @@ -25,18 +25,18 @@ int blk_common_cmd(int argc, char *const argv[], enum uclass_id uclass_id,
>         case 2:
>                 if (strncmp(argv[1], "inf", 3) == 0) {
>                         blk_list_devices(uclass_id);
> -                       return 0;
> +                       return CMD_RET_SUCCESS;

I really don't like this...0 is success.

>                 } else if (strncmp(argv[1], "dev", 3) == 0) {
>                         if (blk_print_device_num(uclass_id, *cur_devnump)) {
>                                 printf("\nno %s devices available\n", if_name);
>                                 return CMD_RET_FAILURE;
>                         }
> -                       return 0;
> +                       return CMD_RET_SUCCESS;
>                 } else if (strncmp(argv[1], "part", 4) == 0) {
>                         if (blk_list_part(uclass_id))
>                                 printf("\nno %s partition table available\n",
>                                        if_name);
> -                       return 0;
> +                       return CMD_RET_SUCCESS;
>                 }
>                 return CMD_RET_USAGE;
>         case 3:
> @@ -49,7 +49,7 @@ int blk_common_cmd(int argc, char *const argv[], enum uclass_id uclass_id,
>                         } else {
>                                 return CMD_RET_FAILURE;
>                         }
> -                       return 0;
> +                       return CMD_RET_SUCCESS;
>                 } else if (strncmp(argv[1], "part", 4) == 0) {
>                         int dev = (int)dectoul(argv[2], NULL);
>
> @@ -58,7 +58,7 @@ int blk_common_cmd(int argc, char *const argv[], enum uclass_id uclass_id,
>                                        if_name, dev);
>                                 return CMD_RET_FAILURE;
>                         }
> -                       return 0;
> +                       return CMD_RET_SUCCESS;
>                 }
>                 return CMD_RET_USAGE;
>
> @@ -80,7 +80,7 @@ int blk_common_cmd(int argc, char *const argv[], enum uclass_id uclass_id,
>
>                         printf("%ld blocks read: %s\n", n,
>                                n == cnt ? "OK" : "ERROR");
> -                       return n == cnt ? 0 : 1;
> +                       return n == cnt ? CMD_RET_SUCCESS : CMD_RET_FAILURE;

CMD_RET_FAILURE is OK, but I would prefer not to use CMD_RET_SUCCESS.
It is 0 and always will be.

It encourages people to do things like:

if (ret == CMD_RET_SUCCESS)

instead of

if (!ret)

It would eventually creep into everything, including our clean error handling.

>                 } else if (strcmp(argv[1], "write") == 0) {
>                         phys_addr_t paddr = hextoul(argv[2], NULL);
>                         lbaint_t blk = hextoul(argv[3], NULL);
> @@ -98,7 +98,7 @@ int blk_common_cmd(int argc, char *const argv[], enum uclass_id uclass_id,
>
>                         printf("%ld blocks written: %s\n", n,
>                                n == cnt ? "OK" : "ERROR");
> -                       return n == cnt ? 0 : 1;
> +                       return n == cnt ? CMD_RET_SUCCESS : CMD_RET_FAILURE;
>                 } else {
>                         return CMD_RET_USAGE;
>                 }
> --
> 2.25.1
>

Regards,
Simon