On Fri, Jul 12, 2019 at 2:21 PM Eugeniu Rosca erosca@de.adit-jv.com wrote:

Commit 9bdf0e8fef86 ("doc: relocate/rename Android README and add BCB overview") left some obsolete references of Android documents/paths.

This has been pointed out by Sam (thanks!) in: https://patchwork.ozlabs.org/patch/1104245/#2208134

Fixes: 9bdf0e8fef86 ("doc: relocate/rename Android README and add BCB overview") Reported-by: Sam Protsenko semen.protsenko@linaro.org Suggested-by: Sam Protsenko semen.protsenko@linaro.org Signed-off-by: Eugeniu Rosca erosca@de.adit-jv.com

---

cmd/Kconfig | 2 +- doc/android/fastboot.txt | 2 +- test/py/tests/test_avb.py | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/cmd/Kconfig b/cmd/Kconfig index 3afb760a816f..3cf8233df62e 100644 --- a/cmd/Kconfig +++ b/cmd/Kconfig @@ -707,7 +707,7 @@ config CMD_FASTBOOT Android devices. Fastboot requires either the network stack enabled or support for acting as a USB device.

•     See doc/README.android-fastboot for more information.
  

•     See doc/android/fastboot.txt for more information.
  
  

config CMD_FDC bool "fdcboot - Boot from floppy device" diff --git a/doc/android/fastboot.txt b/doc/android/fastboot.txt index 431191c473f2..dcf824713f94 100644 --- a/doc/android/fastboot.txt +++ b/doc/android/fastboot.txt @@ -6,7 +6,7 @@ Overview ========

The protocol that is used over USB and UDP is described in the -``README.android-fastboot-protocol`` file in the same directory. +``fastboot-protocol.txt`` file in the same directory.

The current implementation supports the following standard commands:

diff --git a/test/py/tests/test_avb.py b/test/py/tests/test_avb.py index 2bb75ed6e2a2..813242343555 100644 --- a/test/py/tests/test_avb.py +++ b/test/py/tests/test_avb.py @@ -8,7 +8,7 @@ This tests Android Verified Boot 2.0 support in U-boot:

For additional details about how to build proper vbmeta partition -check doc/README.avb2 +check doc/android/avb2.txt

For configuration verification:

• Corrupt boot partition and check for failure

-- 2.22.0

Reviewed-by: Igor Opaniuk igor.opaniuk@gmail.com

Hi Eugeniu,

On Fri, Jul 12, 2019 at 2:21 PM Eugeniu Rosca erosca@de.adit-jv.com wrote:

Quote from https://patchwork.ozlabs.org/patch/1104244/#2210814:

----------8<----------- strncmp() is chosen for the sake of paranoid/defensive programming. Indeed, strncmp() is not really needed when comparing a variable with a string literal. We expect strcmp() to behave safely even if the string variable is not NUL-terminated.

In the same scenario, Linux v5.2-rc7 uses both strcmp() and strncmp(), but the frequency of strcmp() is higher:

$ git --version git version 2.22.0 $ (Linux 5.2-rc7) git grep -En 'strncmp([^"]*"[[:alnum:]]+"' | wc -l 1066 $ (Linux 5.2-rc7) git grep -En 'strcmp([^"]*"[[:alnum:]]+"' | wc -l 1968

A quick "strcmp vs strncmp" object size test shows that strcmp() generates smaller memory footprint (gcc-8, x86_64):

$ (U-Boot) size cmd/bcb-strncmp.o cmd/bcb-strcmp.o text data bss dec hex filename 3373 400 2048 5821 16bd cmd/bcb-strncmp.o 3314 400 2048 5762 1682 cmd/bcb-strcmp.o

So, overall, I agree to use strcmp() whenever variables are compared with string literals. ----------8<-----------

Fixes: db7b7a05b267 ("cmd: Add 'bcb' command to read/modify/write BCB fields") Reported-by: Simon Glass sjg@chromium.org Signed-off-by: Eugeniu Rosca erosca@de.adit-jv.com

---

cmd/bcb.c | 42 +++++++++++++++++++++--------------------- 1 file changed, 21 insertions(+), 21 deletions(-)

diff --git a/cmd/bcb.c b/cmd/bcb.c index 3b1c7434e287..c7138a5179a9 100644 --- a/cmd/bcb.c +++ b/cmd/bcb.c @@ -24,17 +24,17 @@ static struct bootloader_message bcb = { { 0 } };

static int bcb_cmd_get(char *cmd) {

•   if (!strncmp(cmd, "load", sizeof("load")))
  

•   if (!strcmp(cmd, "load"))
            return BCB_CMD_LOAD;
  

•   if (!strncmp(cmd, "set", sizeof("set")))
  

•   if (!strcmp(cmd, "set"))
            return BCB_CMD_FIELD_SET;
  

•   if (!strncmp(cmd, "clear", sizeof("clear")))
  

•   if (!strcmp(cmd, "clear"))
            return BCB_CMD_FIELD_CLEAR;
  

•   if (!strncmp(cmd, "test", sizeof("test")))
  

•   if (!strcmp(cmd, "test"))
            return BCB_CMD_FIELD_TEST;
  

•   if (!strncmp(cmd, "store", sizeof("store")))
  

•   if (!strcmp(cmd, "store"))
            return BCB_CMD_STORE;
  

•   if (!strncmp(cmd, "dump", sizeof("dump")))
  

•   if (!strcmp(cmd, "dump"))
            return BCB_CMD_FIELD_DUMP;
    else
            return -1;
  

@@ -85,21 +85,21 @@ err:

static int bcb_field_get(char *name, char **field, int *size) {

•   if (!strncmp(name, "command", sizeof("command"))) {
  

•           *field = bcb.command;
  

•           *size = sizeof(bcb.command);
  

•   } else if (!strncmp(name, "status", sizeof("status"))) {
  

•           *field = bcb.status;
  

•           *size = sizeof(bcb.status);
  

•   } else if (!strncmp(name, "recovery", sizeof("recovery"))) {
  

•           *field = bcb.recovery;
  

•           *size = sizeof(bcb.recovery);
  

•   } else if (!strncmp(name, "stage", sizeof("stage"))) {
  

•           *field = bcb.stage;
  

•           *size = sizeof(bcb.stage);
  

•   } else if (!strncmp(name, "reserved", sizeof("reserved"))) {
  

•           *field = bcb.reserved;
  

•           *size = sizeof(bcb.reserved);
  

•   if (!strcmp(name, "command")) {
  

•           *fieldp = bcb.command;
  

•           *sizep = sizeof(bcb.command);
  

•   } else if (!strcmp(name, "status")) {
  

•           *fieldp = bcb.status;
  

•           *sizep = sizeof(bcb.status);
  

•   } else if (!strcmp(name, "recovery")) {
  

•           *fieldp = bcb.recovery;
  

•           *sizep = sizeof(bcb.recovery);
  

•   } else if (!strcmp(name, "stage")) {
  

•           *fieldp = bcb.stage;
  

•           *sizep = sizeof(bcb.stage);
  

•   } else if (!strcmp(name, "reserved")) {
  

•           *fieldp = bcb.reserved;
  

•           *sizep = sizeof(bcb.reserved);
    } else {
            printf("Error: Unknown bcb field '%s'\n", name);
            return -1;
  

-- 2.22.0

Reviewed-by: Igor Opaniuk igor.opaniuk@gmail.com

On Fri, Jul 12, 2019 at 2:26 PM Eugeniu Rosca erosca@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.

minor: could you please add more details about particular issues that were addressed in this commit, because current commit message contains only some insights of a glitch in the merge workflow instead of overview of changes.

[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 sjg@chromium.org Signed-off-by: Eugeniu Rosca erosca@de.adit-jv.com

---

cmd/bcb.c | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-)

diff --git a/cmd/bcb.c b/cmd/bcb.c index c7138a5179a9..9626f2c69e34 100644 --- a/cmd/bcb.c +++ b/cmd/bcb.c @@ -83,7 +83,7 @@ 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")) { *fieldp = bcb.command; @@ -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

With my comment addressed: Reviewed-by: Igor Opaniuk igor.opaniuk@gmail.com