On Fri, Jul 12, 2019 at 3:52 PM Eugeniu Rosca <ero...@de.adit-jv.com> wrote:
>
> These have been reported by Simon in [1] and fixed in [2].
> However, since [1] has already been pushed to u-boot/master, the
> improvements incorporated in [2] are now extracted and resubmitted.
>
> The changes are in the area of coding style and best practices:
> * s/field/fieldp/, s/size/sizep/, to convey that the variables return
> an output to the caller
> * s/err_1/err_read_fail/, s/err_2/err_too_small/, to be more descriptive
> * Made sure 'static int do_bcb_load' appears on the same line
> * Placed a `/*` on top of multi-line comment
>
> [1] https://patchwork.ozlabs.org/patch/1104244/#2200259
> [2] https://patchwork.ozlabs.org/cover/1128661/
> ("[v4,0/4] Add 'bcb' command to read/modify/write Android BCB")
>
> Fixes: db7b7a05b267 ("cmd: Add 'bcb' command to read/modify/write BCB fields")
> Reported-by: Simon Glass <s...@chromium.org>
> Signed-off-by: Eugeniu Rosca <ero...@de.adit-jv.com>
> ---
> v2:
> - [Igor Opaniuk] Enriched the patch description
> - Fixed inconsistent {field,size} variable rename
>
> v1:
> - https://patchwork.ozlabs.org/patch/1131321/
> ---
Reviewed-by: Sam Protsenko <semen.protse...@linaro.org>
> cmd/bcb.c | 43 ++++++++++++++++++++++---------------------
> 1 file changed, 22 insertions(+), 21 deletions(-)
>
> diff --git a/cmd/bcb.c b/cmd/bcb.c
> index fa9fdeeb0dfc..9626f2c69e34 100644
> --- a/cmd/bcb.c
> +++ b/cmd/bcb.c
> @@ -83,23 +83,23 @@ err:
> return -1;
> }
>
> -static int bcb_field_get(char *name, char **field, int *size)
> +static int bcb_field_get(char *name, char **fieldp, int *sizep)
> {
> if (!strcmp(name, "command")) {
> - *field = bcb.command;
> - *size = sizeof(bcb.command);
> + *fieldp = bcb.command;
> + *sizep = sizeof(bcb.command);
> } else if (!strcmp(name, "status")) {
> - *field = bcb.status;
> - *size = sizeof(bcb.status);
> + *fieldp = bcb.status;
> + *sizep = sizeof(bcb.status);
> } else if (!strcmp(name, "recovery")) {
> - *field = bcb.recovery;
> - *size = sizeof(bcb.recovery);
> + *fieldp = bcb.recovery;
> + *sizep = sizeof(bcb.recovery);
> } else if (!strcmp(name, "stage")) {
> - *field = bcb.stage;
> - *size = sizeof(bcb.stage);
> + *fieldp = bcb.stage;
> + *sizep = sizeof(bcb.stage);
> } else if (!strcmp(name, "reserved")) {
> - *field = bcb.reserved;
> - *size = sizeof(bcb.reserved);
> + *fieldp = bcb.reserved;
> + *sizep = sizeof(bcb.reserved);
> } else {
> printf("Error: Unknown bcb field '%s'\n", name);
> return -1;
> @@ -108,8 +108,8 @@ static int bcb_field_get(char *name, char **field, int
> *size)
> return 0;
> }
>
> -static int
> -do_bcb_load(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
> +static int do_bcb_load(cmd_tbl_t *cmdtp, int flag, int argc,
> + char * const argv[])
> {
> struct blk_desc *desc;
> disk_partition_t info;
> @@ -119,28 +119,28 @@ do_bcb_load(cmd_tbl_t *cmdtp, int flag, int argc, char
> * const argv[])
>
> ret = blk_get_device_by_str("mmc", argv[1], &desc);
> if (ret < 0)
> - goto err_1;
> + goto err_read_fail;
>
> part = simple_strtoul(argv[2], &endp, 0);
> if (*endp == '\0') {
> ret = part_get_info(desc, part, &info);
> if (ret)
> - goto err_1;
> + goto err_read_fail;
> } else {
> part = part_get_info_by_name(desc, argv[2], &info);
> if (part < 0) {
> ret = part;
> - goto err_1;
> + goto err_read_fail;
> }
> }
>
> cnt = DIV_ROUND_UP(sizeof(struct bootloader_message), info.blksz);
> if (cnt > info.size)
> - goto err_2;
> + goto err_too_small;
>
> if (blk_dread(desc, info.start, cnt, &bcb) != cnt) {
> ret = -EIO;
> - goto err_1;
> + goto err_read_fail;
> }
>
> bcb_dev = desc->devnum;
> @@ -148,10 +148,10 @@ do_bcb_load(cmd_tbl_t *cmdtp, int flag, int argc, char
> * const argv[])
> debug("%s: Loaded from mmc %d:%d\n", __func__, bcb_dev, bcb_part);
>
> return CMD_RET_SUCCESS;
> -err_1:
> +err_read_fail:
> printf("Error: mmc %s:%s read failed (%d)\n", argv[1], argv[2], ret);
> goto err;
> -err_2:
> +err_too_small:
> printf("Error: mmc %s:%s too small!", argv[1], argv[2]);
> goto err;
> err:
> @@ -304,7 +304,8 @@ static int do_bcb(cmd_tbl_t *cmdtp, int flag, int argc,
> char *const argv[])
> return CMD_RET_USAGE;
>
> if (bcb_is_misused(argc, argv)) {
> - /* We try to improve the user experience by reporting the
> + /*
> + * We try to improve the user experience by reporting the
> * root-cause of misusage, so don't return CMD_RET_USAGE,
> * since the latter prints out the full-blown help text
> */
> --
> 2.22.0
>
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
