[PATCH v4 06/10] cmd: ubi: change all positive error return value to negative

[Simon Glass]

Hi Weijie,

On 2026-05-13T08:02:45, Weijie Gao <weijie.gao at mediatek.com> wrote:
> cmd: ubi: change all positive error return value to negative
>
> Change all return value using errno codes to negative. This makes it
> consistent with the linux ubi layer.
>
> Also, to follow the standard definition of U-Boot command, in the do_ubi()
> command handler, the return value is converted to CMD_RET_FAILURE for error
> returning, and CMD_RET_USAGE for incorrect usage.
>
> Signed-off-by: Weijie Gao <weijie.gao at mediatek.com>
>
> cmd/ubi.c | 106 ++++++++++++++++++++++++++++++++++++--------------------------
>  1 file changed, 62 insertions(+), 44 deletions(-)

Reviewed-by: Simon Glass <sjg at chromium.org>

> diff --git a/cmd/ubi.c b/cmd/ubi.c
> @@ -761,7 +762,7 @@ static int do_ubi(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
>                       return ubi_check(argv[2]);
>
>               printf("Error, no volume name passed\n");
> -             return 1;
> +             return CMD_RET_FAILURE;

A missing argument is a usage error, not a runtime failure. Please use
CMD_RET_USAGE here, and likewise for the 'Incorrect type' and 'no
volume name passed' paths in the create handler, so the user sees the
help text.

> diff --git a/cmd/ubi.c b/cmd/ubi.c
> @@ -677,7 +674,7 @@ int ubi_part(const char *part_name, const char *vid_header_offset)
>       mtd = get_mtd_device_nm(part_name);
>       if (IS_ERR(mtd)) {
>               printf("Partition %s not found!\n", part_name);
> -             return 1;
> +             return PTR_ERR(mtd);
>       }

PTR_ERR() returns long but ubi_part() returns int. Please cast, or
just return a specific errno like -ENODEV

> diff --git a/cmd/ubi.c b/cmd/ubi.c
> @@ -806,34 +807,46 @@ static int do_ubi(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
>               }
>               /* E.g., create volume */
>               if (argc == 3) {
> -                     return ubi_create_vol(argv[2], size, dynamic, id,
> -                                           skipcheck);
> +                     ret = ubi_create_vol(argv[2], size, dynamic, id,
> +                                          skipcheck);
> +                     if (ret)
> +                             return CMD_RET_FAILURE;
> +                     return 0;
>               }

This three-line conversion is repeated about seven times in do_ubi().
Since the called functions already print their own errors, a helper or
just 'return ret ? CMD_RET_FAILURE : 0;' would cut the duplication.
What do you think?

Regards,
Simon