| Message ID | 20230714112451.144294-1-matthias.schiffer@ew.tq-group.com |
|---|---|
| State | Accepted |
| Commit | 615828721abfe8c73b5103d4436402ecbf9b9897 |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | [1/2] Revert "lib: string: Fix strlcpy return value", fix callers | expand |
On Fri, 2023-07-14 at 13:24 +0200, Matthias Schiffer wrote: > Both the Linux kernel and libbsd agree that strlcpy() should always > return strlen(src) and not include the NUL termination. The incorrect > U-Boot implementation makes it impossible to check the return value for > truncation, and breaks code written with the usual implementation in > mind (for example, fdtdec_add_reserved_memory() was subtly broken). > > I reviewed all callers of strlcpy() and strlcat() and fixed them > according to my understanding of the intended function. > > This reverts commit d3358ecc54be0bc3b4dd11f7a63eab0a2842f772 and adds > related fixes. > > Fixes: d3358ecc54be ("lib: string: Fix strlcpy return value") > Signed-off-by: Matthias Schiffer <matthias.schiffer@ew.tq-group.com> Ping~ strlcpy and strlcat are now also in glibc and might be added to POSIX, so it would be great if we could get the U-Boot implementation to match the common behaviour. Regards, Matthias > --- > board/amlogic/vim3/vim3.c | 6 +++--- > drivers/fastboot/fb_getvar.c | 2 +- > lib/string.c | 14 +++++++------- > test/lib/strlcat.c | 4 ++-- > 4 files changed, 13 insertions(+), 13 deletions(-) > > diff --git a/board/amlogic/vim3/vim3.c b/board/amlogic/vim3/vim3.c > index fcd60ab1e05..8bdfb302f72 100644 > --- a/board/amlogic/vim3/vim3.c > +++ b/board/amlogic/vim3/vim3.c > @@ -104,8 +104,8 @@ int meson_ft_board_setup(void *blob, struct bd_info *bd) > } > > /* Update PHY names (mandatory to disable USB3.0) */ > - len = strlcpy(data, "usb2-phy0", 32); > - len += strlcpy(&data[len], "usb2-phy1", 32 - len); > + len = strlcpy(data, "usb2-phy0", 32) + 1; > + len += strlcpy(&data[len], "usb2-phy1", 32 - len) + 1; > ret = fdt_setprop(blob, node, "phy-names", data, len); > if (ret < 0) { > printf("vim3: failed to update usb phy names property (%d)\n", ret); > @@ -132,7 +132,7 @@ int meson_ft_board_setup(void *blob, struct bd_info *bd) > } > > /* Enable PCIe */ > - len = strlcpy(data, "okay", 32); > + len = strlcpy(data, "okay", 32) + 1; > ret = fdt_setprop(blob, node, "status", data, len); > if (ret < 0) { > printf("vim3: failed to enable pcie node (%d)\n", ret); > diff --git a/drivers/fastboot/fb_getvar.c b/drivers/fastboot/fb_getvar.c > index dd3475e0a8b..d9f0f07b2bc 100644 > --- a/drivers/fastboot/fb_getvar.c > +++ b/drivers/fastboot/fb_getvar.c > @@ -183,7 +183,7 @@ static void __maybe_unused getvar_has_slot(char *part_name, char *response) > > /* part_name_wslot = part_name + "_a" */ > len = strlcpy(part_name_wslot, part_name, PART_NAME_LEN - 3); > - if (len > PART_NAME_LEN - 3) > + if (len >= PART_NAME_LEN - 3) > goto fail; > strcat(part_name_wslot, "_a"); > > diff --git a/lib/string.c b/lib/string.c > index ecea755f405..f2c61471288 100644 > --- a/lib/string.c > +++ b/lib/string.c > @@ -116,20 +116,18 @@ char * strncpy(char * dest,const char *src,size_t count) > * of course, the buffer size is zero). It does not pad > * out the result like strncpy() does. > * > - * Return: the number of bytes copied > + * Return: strlen(src) > */ > size_t strlcpy(char *dest, const char *src, size_t size) > { > - if (size) { > - size_t srclen = strlen(src); > - size_t len = (srclen >= size) ? size - 1 : srclen; > + size_t ret = strlen(src); > > + if (size) { > + size_t len = (ret >= size) ? size - 1 : ret; > memcpy(dest, src, len); > dest[len] = '\0'; > - return len + 1; > } > - > - return 0; > + return ret; > } > #endif > > @@ -191,6 +189,8 @@ char * strncat(char *dest, const char *src, size_t count) > * Compatible with *BSD: the result is always a valid NUL-terminated string that > * fits in the buffer (unless, of course, the buffer size is zero). It does not > * write past @size like strncat() does. > + * > + * Return: min(strlen(dest), size) + strlen(src) > */ > size_t strlcat(char *dest, const char *src, size_t size) > { > diff --git a/test/lib/strlcat.c b/test/lib/strlcat.c > index a0ec037388b..d8453fe78e2 100644 > --- a/test/lib/strlcat.c > +++ b/test/lib/strlcat.c > @@ -43,11 +43,11 @@ static int do_test_strlcat(struct unit_test_state *uts, int line, size_t align1, > s2[i] = 32 + 23 * i % (127 - 32); > s2[len2 - 1] = '\0'; > > - expected = len2 < n ? min(len1 + len2 - 1, n) : n; > + expected = min(strlen(s2), n) + strlen(s1); > actual = strlcat(s2, s1, n); > if (expected != actual) { > ut_failf(uts, __FILE__, line, __func__, > - "strlcat(s2, s1, 2) == len2 < n ? min(len1 + len2, n) : n", > + "strlcat(s2, s1, n) == min(len2, n) + len1", > "Expected %#zx (%zd), got %#zx (%zd)", > expected, expected, actual, actual); > return CMD_RET_FAILURE;
On Wed, Aug 02, 2023 at 12:06:26PM +0200, Matthias Schiffer wrote: > On Fri, 2023-07-14 at 13:24 +0200, Matthias Schiffer wrote: > > Both the Linux kernel and libbsd agree that strlcpy() should always > > return strlen(src) and not include the NUL termination. The incorrect > > U-Boot implementation makes it impossible to check the return value for > > truncation, and breaks code written with the usual implementation in > > mind (for example, fdtdec_add_reserved_memory() was subtly broken). > > > > I reviewed all callers of strlcpy() and strlcat() and fixed them > > according to my understanding of the intended function. > > > > This reverts commit d3358ecc54be0bc3b4dd11f7a63eab0a2842f772 and adds > > related fixes. > > > > Fixes: d3358ecc54be ("lib: string: Fix strlcpy return value") > > Signed-off-by: Matthias Schiffer <matthias.schiffer@ew.tq-group.com> > > Ping~ > > strlcpy and strlcat are now also in glibc and might be added to POSIX, so it would be great if we > could get the U-Boot implementation to match the common behaviour. I intend to pull this in to -next when I open it post -rc2, thanks.
Hi Matthias, On Fri, 14 Jul 2023 at 05:25, Matthias Schiffer <matthias.schiffer@ew.tq-group.com> wrote: > > Both the Linux kernel and libbsd agree that strlcpy() should always > return strlen(src) and not include the NUL termination. The incorrect > U-Boot implementation makes it impossible to check the return value for > truncation, and breaks code written with the usual implementation in > mind (for example, fdtdec_add_reserved_memory() was subtly broken). > > I reviewed all callers of strlcpy() and strlcat() and fixed them > according to my understanding of the intended function. > > This reverts commit d3358ecc54be0bc3b4dd11f7a63eab0a2842f772 and adds > related fixes. > > Fixes: d3358ecc54be ("lib: string: Fix strlcpy return value") > Signed-off-by: Matthias Schiffer <matthias.schiffer@ew.tq-group.com> > --- > board/amlogic/vim3/vim3.c | 6 +++--- > drivers/fastboot/fb_getvar.c | 2 +- > lib/string.c | 14 +++++++------- > test/lib/strlcat.c | 4 ++-- > 4 files changed, 13 insertions(+), 13 deletions(-) Reviewed-by: Simon Glass <sjg@chromium.org> nit below > > diff --git a/board/amlogic/vim3/vim3.c b/board/amlogic/vim3/vim3.c > index fcd60ab1e05..8bdfb302f72 100644 > --- a/board/amlogic/vim3/vim3.c > +++ b/board/amlogic/vim3/vim3.c > @@ -104,8 +104,8 @@ int meson_ft_board_setup(void *blob, struct bd_info *bd) > } > > /* Update PHY names (mandatory to disable USB3.0) */ > - len = strlcpy(data, "usb2-phy0", 32); > - len += strlcpy(&data[len], "usb2-phy1", 32 - len); > + len = strlcpy(data, "usb2-phy0", 32) + 1; > + len += strlcpy(&data[len], "usb2-phy1", 32 - len) + 1; > ret = fdt_setprop(blob, node, "phy-names", data, len); > if (ret < 0) { > printf("vim3: failed to update usb phy names property (%d)\n", ret); > @@ -132,7 +132,7 @@ int meson_ft_board_setup(void *blob, struct bd_info *bd) > } > > /* Enable PCIe */ > - len = strlcpy(data, "okay", 32); > + len = strlcpy(data, "okay", 32) + 1; > ret = fdt_setprop(blob, node, "status", data, len); > if (ret < 0) { > printf("vim3: failed to enable pcie node (%d)\n", ret); > diff --git a/drivers/fastboot/fb_getvar.c b/drivers/fastboot/fb_getvar.c > index dd3475e0a8b..d9f0f07b2bc 100644 > --- a/drivers/fastboot/fb_getvar.c > +++ b/drivers/fastboot/fb_getvar.c > @@ -183,7 +183,7 @@ static void __maybe_unused getvar_has_slot(char *part_name, char *response) > > /* part_name_wslot = part_name + "_a" */ > len = strlcpy(part_name_wslot, part_name, PART_NAME_LEN - 3); > - if (len > PART_NAME_LEN - 3) > + if (len >= PART_NAME_LEN - 3) > goto fail; > strcat(part_name_wslot, "_a"); > > diff --git a/lib/string.c b/lib/string.c > index ecea755f405..f2c61471288 100644 > --- a/lib/string.c > +++ b/lib/string.c > @@ -116,20 +116,18 @@ char * strncpy(char * dest,const char *src,size_t count) > * of course, the buffer size is zero). It does not pad > * out the result like strncpy() does. > * > - * Return: the number of bytes copied > + * Return: strlen(src) > */ > size_t strlcpy(char *dest, const char *src, size_t size) > { > - if (size) { > - size_t srclen = strlen(src); > - size_t len = (srclen >= size) ? size - 1 : srclen; > + size_t ret = strlen(src); > > + if (size) { > + size_t len = (ret >= size) ? size - 1 : ret; blank line here > memcpy(dest, src, len); > dest[len] = '\0'; > - return len + 1; > } > - > - return 0; > + return ret; > } > #endif > > @@ -191,6 +189,8 @@ char * strncat(char *dest, const char *src, size_t count) > * Compatible with *BSD: the result is always a valid NUL-terminated string that > * fits in the buffer (unless, of course, the buffer size is zero). It does not > * write past @size like strncat() does. > + * > + * Return: min(strlen(dest), size) + strlen(src) > */ > size_t strlcat(char *dest, const char *src, size_t size) > { > diff --git a/test/lib/strlcat.c b/test/lib/strlcat.c > index a0ec037388b..d8453fe78e2 100644 > --- a/test/lib/strlcat.c > +++ b/test/lib/strlcat.c > @@ -43,11 +43,11 @@ static int do_test_strlcat(struct unit_test_state *uts, int line, size_t align1, > s2[i] = 32 + 23 * i % (127 - 32); > s2[len2 - 1] = '\0'; > > - expected = len2 < n ? min(len1 + len2 - 1, n) : n; > + expected = min(strlen(s2), n) + strlen(s1); > actual = strlcat(s2, s1, n); > if (expected != actual) { > ut_failf(uts, __FILE__, line, __func__, > - "strlcat(s2, s1, 2) == len2 < n ? min(len1 + len2, n) : n", > + "strlcat(s2, s1, n) == min(len2, n) + len1", > "Expected %#zx (%zd), got %#zx (%zd)", > expected, expected, actual, actual); > return CMD_RET_FAILURE; > -- > TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany > Amtsgericht München, HRB 105018 > Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider > https://www.tq-group.com/ > Regards, Simon
On 7/14/23 07:24, Matthias Schiffer wrote: > Both the Linux kernel and libbsd agree that strlcpy() should always > return strlen(src) and not include the NUL termination. The incorrect > U-Boot implementation makes it impossible to check the return value for > truncation, and breaks code written with the usual implementation in > mind (for example, fdtdec_add_reserved_memory() was subtly broken). > > I reviewed all callers of strlcpy() and strlcat() and fixed them > according to my understanding of the intended function. > > This reverts commit d3358ecc54be0bc3b4dd11f7a63eab0a2842f772 and adds > related fixes. > > Fixes: d3358ecc54be ("lib: string: Fix strlcpy return value") > Signed-off-by: Matthias Schiffer <matthias.schiffer@ew.tq-group.com> > --- > board/amlogic/vim3/vim3.c | 6 +++--- > drivers/fastboot/fb_getvar.c | 2 +- > lib/string.c | 14 +++++++------- > test/lib/strlcat.c | 4 ++-- > 4 files changed, 13 insertions(+), 13 deletions(-) > > diff --git a/board/amlogic/vim3/vim3.c b/board/amlogic/vim3/vim3.c > index fcd60ab1e05..8bdfb302f72 100644 > --- a/board/amlogic/vim3/vim3.c > +++ b/board/amlogic/vim3/vim3.c > @@ -104,8 +104,8 @@ int meson_ft_board_setup(void *blob, struct bd_info *bd) > } > > /* Update PHY names (mandatory to disable USB3.0) */ > - len = strlcpy(data, "usb2-phy0", 32); > - len += strlcpy(&data[len], "usb2-phy1", 32 - len); > + len = strlcpy(data, "usb2-phy0", 32) + 1; > + len += strlcpy(&data[len], "usb2-phy1", 32 - len) + 1; > ret = fdt_setprop(blob, node, "phy-names", data, len); > if (ret < 0) { > printf("vim3: failed to update usb phy names property (%d)\n", ret); > @@ -132,7 +132,7 @@ int meson_ft_board_setup(void *blob, struct bd_info *bd) > } > > /* Enable PCIe */ > - len = strlcpy(data, "okay", 32); > + len = strlcpy(data, "okay", 32) + 1; > ret = fdt_setprop(blob, node, "status", data, len); > if (ret < 0) { > printf("vim3: failed to enable pcie node (%d)\n", ret); > diff --git a/drivers/fastboot/fb_getvar.c b/drivers/fastboot/fb_getvar.c > index dd3475e0a8b..d9f0f07b2bc 100644 > --- a/drivers/fastboot/fb_getvar.c > +++ b/drivers/fastboot/fb_getvar.c > @@ -183,7 +183,7 @@ static void __maybe_unused getvar_has_slot(char *part_name, char *response) > > /* part_name_wslot = part_name + "_a" */ > len = strlcpy(part_name_wslot, part_name, PART_NAME_LEN - 3); > - if (len > PART_NAME_LEN - 3) > + if (len >= PART_NAME_LEN - 3) > goto fail; > strcat(part_name_wslot, "_a"); > > diff --git a/lib/string.c b/lib/string.c > index ecea755f405..f2c61471288 100644 > --- a/lib/string.c > +++ b/lib/string.c > @@ -116,20 +116,18 @@ char * strncpy(char * dest,const char *src,size_t count) > * of course, the buffer size is zero). It does not pad > * out the result like strncpy() does. > * > - * Return: the number of bytes copied > + * Return: strlen(src) > */ > size_t strlcpy(char *dest, const char *src, size_t size) > { > - if (size) { > - size_t srclen = strlen(src); > - size_t len = (srclen >= size) ? size - 1 : srclen; > + size_t ret = strlen(src); > > + if (size) { > + size_t len = (ret >= size) ? size - 1 : ret; > memcpy(dest, src, len); > dest[len] = '\0'; > - return len + 1; > } > - > - return 0; > + return ret; > } > #endif > > @@ -191,6 +189,8 @@ char * strncat(char *dest, const char *src, size_t count) > * Compatible with *BSD: the result is always a valid NUL-terminated string that > * fits in the buffer (unless, of course, the buffer size is zero). It does not > * write past @size like strncat() does. > + * > + * Return: min(strlen(dest), size) + strlen(src) > */ > size_t strlcat(char *dest, const char *src, size_t size) > { > diff --git a/test/lib/strlcat.c b/test/lib/strlcat.c > index a0ec037388b..d8453fe78e2 100644 > --- a/test/lib/strlcat.c > +++ b/test/lib/strlcat.c > @@ -43,11 +43,11 @@ static int do_test_strlcat(struct unit_test_state *uts, int line, size_t align1, > s2[i] = 32 + 23 * i % (127 - 32); > s2[len2 - 1] = '\0'; > > - expected = len2 < n ? min(len1 + len2 - 1, n) : n; > + expected = min(strlen(s2), n) + strlen(s1); > actual = strlcat(s2, s1, n); > if (expected != actual) { > ut_failf(uts, __FILE__, line, __func__, > - "strlcat(s2, s1, 2) == len2 < n ? min(len1 + len2, n) : n", > + "strlcat(s2, s1, n) == min(len2, n) + len1", > "Expected %#zx (%zd), got %#zx (%zd)", > expected, expected, actual, actual); > return CMD_RET_FAILURE; I remembered that something was off with this patch, but I never went back to fix it... Reviewed-by: Sean Anderson <sean.anderson@seco.com>
On 14/07/2023 13.24, Matthias Schiffer wrote: > Both the Linux kernel and libbsd agree that strlcpy() should always > return strlen(src) and not include the NUL termination. The incorrect > U-Boot implementation makes it impossible to check the return value for > truncation, and breaks code written with the usual implementation in > mind (for example, fdtdec_add_reserved_memory() was subtly broken). So while we're fixing non-standard string function behaviour, can we _please_ finally fix strncpy() so it follows C/POSIX? https://lore.kernel.org/u-boot/52bc92d4-8651-48df-3577-043aa2e16722@prevas.dk/ Note in particular the very last paragraph. To rephrase and give a concrete example, what I'm worried about is us importing some file system code that (correctly) uses strncpy() to fully initialize some memory buffer, then writes that out to disk - but with our crippled strncpy(), the result is potential garbage on-disk, which both our own read implementation (which is likely also just copied) and the kernel's may subsequently choke on. Correctness first, please. If there are any performance problems, those should be identified individually and perhaps rewritten to not use strncpy() after verifying that not zeroing the tail is ok for that call site. Rasmus
On Fri, Jul 14, 2023 at 01:24:50PM +0200, Matthias Schiffer wrote: > Both the Linux kernel and libbsd agree that strlcpy() should always > return strlen(src) and not include the NUL termination. The incorrect > U-Boot implementation makes it impossible to check the return value for > truncation, and breaks code written with the usual implementation in > mind (for example, fdtdec_add_reserved_memory() was subtly broken). > > I reviewed all callers of strlcpy() and strlcat() and fixed them > according to my understanding of the intended function. > > This reverts commit d3358ecc54be0bc3b4dd11f7a63eab0a2842f772 and adds > related fixes. > > Fixes: d3358ecc54be ("lib: string: Fix strlcpy return value") > Signed-off-by: Matthias Schiffer <matthias.schiffer@ew.tq-group.com> > Reviewed-by: Simon Glass <sjg@chromium.org> > Reviewed-by: Sean Anderson <sean.anderson@seco.com> Applied to u-boot/next, thanks!
On Mon, Aug 07, 2023 at 09:45:37AM +0200, Rasmus Villemoes wrote: > On 14/07/2023 13.24, Matthias Schiffer wrote: > > Both the Linux kernel and libbsd agree that strlcpy() should always > > return strlen(src) and not include the NUL termination. The incorrect > > U-Boot implementation makes it impossible to check the return value for > > truncation, and breaks code written with the usual implementation in > > mind (for example, fdtdec_add_reserved_memory() was subtly broken). > > So while we're fixing non-standard string function behaviour, can we > _please_ finally fix strncpy() so it follows C/POSIX? > > https://lore.kernel.org/u-boot/52bc92d4-8651-48df-3577-043aa2e16722@prevas.dk/ > > Note in particular the very last paragraph. > > To rephrase and give a concrete example, what I'm worried about is us > importing some file system code that (correctly) uses strncpy() to fully > initialize some memory buffer, then writes that out to disk - but with > our crippled strncpy(), the result is potential garbage on-disk, which > both our own read implementation (which is likely also just copied) and > the kernel's may subsequently choke on. > > Correctness first, please. If there are any performance problems, those > should be identified individually and perhaps rewritten to not use > strncpy() after verifying that not zeroing the tail is ok for that call > site. Yes, I agree it would be good to make this change.
diff --git a/board/amlogic/vim3/vim3.c b/board/amlogic/vim3/vim3.c index fcd60ab1e05..8bdfb302f72 100644 --- a/board/amlogic/vim3/vim3.c +++ b/board/amlogic/vim3/vim3.c @@ -104,8 +104,8 @@ int meson_ft_board_setup(void *blob, struct bd_info *bd) } /* Update PHY names (mandatory to disable USB3.0) */ - len = strlcpy(data, "usb2-phy0", 32); - len += strlcpy(&data[len], "usb2-phy1", 32 - len); + len = strlcpy(data, "usb2-phy0", 32) + 1; + len += strlcpy(&data[len], "usb2-phy1", 32 - len) + 1; ret = fdt_setprop(blob, node, "phy-names", data, len); if (ret < 0) { printf("vim3: failed to update usb phy names property (%d)\n", ret); @@ -132,7 +132,7 @@ int meson_ft_board_setup(void *blob, struct bd_info *bd) } /* Enable PCIe */ - len = strlcpy(data, "okay", 32); + len = strlcpy(data, "okay", 32) + 1; ret = fdt_setprop(blob, node, "status", data, len); if (ret < 0) { printf("vim3: failed to enable pcie node (%d)\n", ret); diff --git a/drivers/fastboot/fb_getvar.c b/drivers/fastboot/fb_getvar.c index dd3475e0a8b..d9f0f07b2bc 100644 --- a/drivers/fastboot/fb_getvar.c +++ b/drivers/fastboot/fb_getvar.c @@ -183,7 +183,7 @@ static void __maybe_unused getvar_has_slot(char *part_name, char *response) /* part_name_wslot = part_name + "_a" */ len = strlcpy(part_name_wslot, part_name, PART_NAME_LEN - 3); - if (len > PART_NAME_LEN - 3) + if (len >= PART_NAME_LEN - 3) goto fail; strcat(part_name_wslot, "_a"); diff --git a/lib/string.c b/lib/string.c index ecea755f405..f2c61471288 100644 --- a/lib/string.c +++ b/lib/string.c @@ -116,20 +116,18 @@ char * strncpy(char * dest,const char *src,size_t count) * of course, the buffer size is zero). It does not pad * out the result like strncpy() does. * - * Return: the number of bytes copied + * Return: strlen(src) */ size_t strlcpy(char *dest, const char *src, size_t size) { - if (size) { - size_t srclen = strlen(src); - size_t len = (srclen >= size) ? size - 1 : srclen; + size_t ret = strlen(src); + if (size) { + size_t len = (ret >= size) ? size - 1 : ret; memcpy(dest, src, len); dest[len] = '\0'; - return len + 1; } - - return 0; + return ret; } #endif @@ -191,6 +189,8 @@ char * strncat(char *dest, const char *src, size_t count) * Compatible with *BSD: the result is always a valid NUL-terminated string that * fits in the buffer (unless, of course, the buffer size is zero). It does not * write past @size like strncat() does. + * + * Return: min(strlen(dest), size) + strlen(src) */ size_t strlcat(char *dest, const char *src, size_t size) { diff --git a/test/lib/strlcat.c b/test/lib/strlcat.c index a0ec037388b..d8453fe78e2 100644 --- a/test/lib/strlcat.c +++ b/test/lib/strlcat.c @@ -43,11 +43,11 @@ static int do_test_strlcat(struct unit_test_state *uts, int line, size_t align1, s2[i] = 32 + 23 * i % (127 - 32); s2[len2 - 1] = '\0'; - expected = len2 < n ? min(len1 + len2 - 1, n) : n; + expected = min(strlen(s2), n) + strlen(s1); actual = strlcat(s2, s1, n); if (expected != actual) { ut_failf(uts, __FILE__, line, __func__, - "strlcat(s2, s1, 2) == len2 < n ? min(len1 + len2, n) : n", + "strlcat(s2, s1, n) == min(len2, n) + len1", "Expected %#zx (%zd), got %#zx (%zd)", expected, expected, actual, actual); return CMD_RET_FAILURE;
Both the Linux kernel and libbsd agree that strlcpy() should always return strlen(src) and not include the NUL termination. The incorrect U-Boot implementation makes it impossible to check the return value for truncation, and breaks code written with the usual implementation in mind (for example, fdtdec_add_reserved_memory() was subtly broken). I reviewed all callers of strlcpy() and strlcat() and fixed them according to my understanding of the intended function. This reverts commit d3358ecc54be0bc3b4dd11f7a63eab0a2842f772 and adds related fixes. Fixes: d3358ecc54be ("lib: string: Fix strlcpy return value") Signed-off-by: Matthias Schiffer <matthias.schiffer@ew.tq-group.com> --- board/amlogic/vim3/vim3.c | 6 +++--- drivers/fastboot/fb_getvar.c | 2 +- lib/string.c | 14 +++++++------- test/lib/strlcat.c | 4 ++-- 4 files changed, 13 insertions(+), 13 deletions(-)