Hi Weijie, On 2026-05-13T08:02:45, Weijie Gao <[email protected]> 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 <[email protected]> > > cmd/ubi.c | 106 ++++++++++++++++++++++++++++++++++++-------------------------- > 1 file changed, 62 insertions(+), 44 deletions(-)
Reviewed-by: Simon Glass <[email protected]> > 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
