[PATCH 0/3] lib: uuid: fix uuid_str_to_le_bin() on 32-bit
The lib_test_uuid_to_le test fails on 32-bit systems. But we never caught this in our CI because we never ran any of our C unit tests on 32-bit.
Enable CONFIG_UNIT_TEST on qemu_arm_defconfig.
hextoul() cannot convert a string to a 64-bit number on a 32-bit system. Use the new function hextoull() instead.
Heinrich Schuchardt (3): lib: provide function hextoull() lib: uuid: fix uuid_str_to_le_bin() on 32-bit configs: enable UNIT_TEST on qemu_arm_defconfig
configs/qemu_arm_defconfig | 1 + include/vsprintf.h | 13 +++++++++++++ lib/strto.c | 5 +++++ lib/uuid.c | 3 ++- 4 files changed, 21 insertions(+), 1 deletion(-)
We often convert hexadecimal strings to hextoull(). Provide a wrapper function to simple_strtoull() that does not require specifying the radix.
Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com --- include/vsprintf.h | 13 +++++++++++++ lib/strto.c | 5 +++++ 2 files changed, 18 insertions(+)
diff --git a/include/vsprintf.h b/include/vsprintf.h index fe951471426..9da6ce7cc4d 100644 --- a/include/vsprintf.h +++ b/include/vsprintf.h @@ -44,6 +44,19 @@ ulong simple_strtoul(const char *cp, char **endp, unsigned int base); */ unsigned long hextoul(const char *cp, char **endp);
+/** + * hex_strtoull - convert a string in hex to an unsigned long long + * + * @cp: The string to be converted + * @endp: Updated to point to the first character not converted + * Return: value decoded from string (0 if invalid) + * + * Converts a hex string to an unsigned long long. If there are invalid + * characters at the end these are ignored. In the worst case, if all characters + * are invalid, 0 is returned + */ +unsigned long long hextoull(const char *cp, char **endp); + /** * dec_strtoul - convert a string in decimal to an unsigned long * diff --git a/lib/strto.c b/lib/strto.c index f83ac67c666..206d1e91847 100644 --- a/lib/strto.c +++ b/lib/strto.c @@ -78,6 +78,11 @@ ulong hextoul(const char *cp, char **endp) return simple_strtoul(cp, endp, 16); }
+unsigned long long hextoull(const char *cp, char **endp) +{ + return simple_strtoull(cp, endp, 16); +} + ulong dectoul(const char *cp, char **endp) { return simple_strtoul(cp, endp, 10);
hextoul() cannot convert a string to a 64-bit number on a 32-bit system. Use function hextoull() instead.
Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com --- lib/uuid.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/lib/uuid.c b/lib/uuid.c index c6a27b7d044..19f33dd1477 100644 --- a/lib/uuid.c +++ b/lib/uuid.c @@ -35,6 +35,7 @@ #ifdef USE_HOSTCC /* polyfill hextoul to avoid pulling in strto.c */ #define hextoul(cp, endp) strtoul(cp, endp, 16) +#define hextoull(cp, endp) strtoull(cp, endp, 16) #endif
int uuid_str_valid(const char *uuid) @@ -339,7 +340,7 @@ int uuid_str_to_le_bin(const char *uuid_str, unsigned char *uuid_bin) tmp16 = cpu_to_le16(hextoul(uuid_str + 19, NULL)); memcpy(uuid_bin + 8, &tmp16, 2);
- tmp64 = cpu_to_le64(hextoul(uuid_str + 24, NULL)); + tmp64 = cpu_to_le64(hextoull(uuid_str + 24, NULL)); memcpy(uuid_bin + 10, &tmp64, 6);
return 0;
On Sun, Nov 03, 2024 at 08:11:59AM +0100, Heinrich Schuchardt wrote:
hextoul() cannot convert a string to a 64-bit number on a 32-bit system. Use function hextoull() instead.
Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com
Reported-by: Patrick Delaunay patrick.delaunay@foss.st.com Fixes: 22c48a92cdce ("lib: uuid: supporting building as part of host tools")
lib/uuid.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/lib/uuid.c b/lib/uuid.c index c6a27b7d044..19f33dd1477 100644 --- a/lib/uuid.c +++ b/lib/uuid.c @@ -35,6 +35,7 @@ #ifdef USE_HOSTCC /* polyfill hextoul to avoid pulling in strto.c */ #define hextoul(cp, endp) strtoul(cp, endp, 16) +#define hextoull(cp, endp) strtoull(cp, endp, 16) #endif
int uuid_str_valid(const char *uuid) @@ -339,7 +340,7 @@ int uuid_str_to_le_bin(const char *uuid_str, unsigned char *uuid_bin) tmp16 = cpu_to_le16(hextoul(uuid_str + 19, NULL)); memcpy(uuid_bin + 8, &tmp16, 2);
- tmp64 = cpu_to_le64(hextoul(uuid_str + 24, NULL));
tmp64 = cpu_to_le64(hextoull(uuid_str + 24, NULL)); memcpy(uuid_bin + 10, &tmp64, 6);
return 0;
-- 2.45.2
Thanks Heinrich
On Sun, 3 Nov 2024 at 07:12, Heinrich Schuchardt heinrich.schuchardt@canonical.com wrote:
hextoul() cannot convert a string to a 64-bit number on a 32-bit system. Use function hextoull() instead.
Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com
lib/uuid.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/lib/uuid.c b/lib/uuid.c index c6a27b7d044..19f33dd1477 100644 --- a/lib/uuid.c +++ b/lib/uuid.c @@ -35,6 +35,7 @@ #ifdef USE_HOSTCC /* polyfill hextoul to avoid pulling in strto.c */ #define hextoul(cp, endp) strtoul(cp, endp, 16) +#define hextoull(cp, endp) strtoull(cp, endp, 16) #endif
int uuid_str_valid(const char *uuid) @@ -339,7 +340,7 @@ int uuid_str_to_le_bin(const char *uuid_str, unsigned char *uuid_bin) tmp16 = cpu_to_le16(hextoul(uuid_str + 19, NULL)); memcpy(uuid_bin + 8, &tmp16, 2);
tmp64 = cpu_to_le64(hextoul(uuid_str + 24, NULL));
tmp64 = cpu_to_le64(hextoull(uuid_str + 24, NULL)); memcpy(uuid_bin + 10, &tmp64, 6); return 0;-- 2.45.2
Reviewed-by: Ilias Apalodimas ilias.apalodimas@linaro.org
The lib_test_uuid_to_le test fails on 32-bit systems. But we never caught this in our CI because we never ran any of our C unit tests on 32-bit.
Enable CONFIG_UNIT_TEST on qemu_arm_defconfig.
Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com --- configs/qemu_arm_defconfig | 1 + 1 file changed, 1 insertion(+)
diff --git a/configs/qemu_arm_defconfig b/configs/qemu_arm_defconfig index d042aea49bb..cc4f4540fd5 100644 --- a/configs/qemu_arm_defconfig +++ b/configs/qemu_arm_defconfig @@ -67,3 +67,4 @@ CONFIG_TPM2_MMIO=y CONFIG_USB_EHCI_HCD=y CONFIG_USB_EHCI_PCI=y CONFIG_TPM=y +CONFIG_UNIT_TEST=y
On 11/3/24 08:12, Heinrich Schuchardt wrote:
The lib_test_uuid_to_le test fails on 32-bit systems. But we never caught this in our CI because we never ran any of our C unit tests on 32-bit.
Enable CONFIG_UNIT_TEST on qemu_arm_defconfig.
Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com
configs/qemu_arm_defconfig | 1 + 1 file changed, 1 insertion(+)
diff --git a/configs/qemu_arm_defconfig b/configs/qemu_arm_defconfig index d042aea49bb..cc4f4540fd5 100644 --- a/configs/qemu_arm_defconfig +++ b/configs/qemu_arm_defconfig @@ -67,3 +67,4 @@ CONFIG_TPM2_MMIO=y CONFIG_USB_EHCI_HCD=y CONFIG_USB_EHCI_PCI=y CONFIG_TPM=y +CONFIG_UNIT_TEST=y
Before merging this patch we also have to fix the lib_test_dynamic_uuid which fails on 32-bit.
Best regards
Heinrich
On Sun, Nov 03, 2024 at 04:45:41PM +0100, Heinrich Schuchardt wrote:
On 11/3/24 08:12, Heinrich Schuchardt wrote:
The lib_test_uuid_to_le test fails on 32-bit systems. But we never caught this in our CI because we never ran any of our C unit tests on 32-bit.
Enable CONFIG_UNIT_TEST on qemu_arm_defconfig.
Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com
configs/qemu_arm_defconfig | 1 + 1 file changed, 1 insertion(+)
diff --git a/configs/qemu_arm_defconfig b/configs/qemu_arm_defconfig index d042aea49bb..cc4f4540fd5 100644 --- a/configs/qemu_arm_defconfig +++ b/configs/qemu_arm_defconfig @@ -67,3 +67,4 @@ CONFIG_TPM2_MMIO=y CONFIG_USB_EHCI_HCD=y CONFIG_USB_EHCI_PCI=y CONFIG_TPM=y +CONFIG_UNIT_TEST=y
Before merging this patch we also have to fix the lib_test_dynamic_uuid which fails on 32-bit.
Is it the test, or is it the UUID changes? I didn't go so far as to revert the UUID changes when I hit this on Pi 3 32b, just poked Caleb on irc.
On 11/3/24 16:50, Tom Rini wrote:
On Sun, Nov 03, 2024 at 04:45:41PM +0100, Heinrich Schuchardt wrote:
On 11/3/24 08:12, Heinrich Schuchardt wrote:
The lib_test_uuid_to_le test fails on 32-bit systems. But we never caught this in our CI because we never ran any of our C unit tests on 32-bit.
Enable CONFIG_UNIT_TEST on qemu_arm_defconfig.
Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com
configs/qemu_arm_defconfig | 1 + 1 file changed, 1 insertion(+)
diff --git a/configs/qemu_arm_defconfig b/configs/qemu_arm_defconfig index d042aea49bb..cc4f4540fd5 100644 --- a/configs/qemu_arm_defconfig +++ b/configs/qemu_arm_defconfig @@ -67,3 +67,4 @@ CONFIG_TPM2_MMIO=y CONFIG_USB_EHCI_HCD=y CONFIG_USB_EHCI_PCI=y CONFIG_TPM=y +CONFIG_UNIT_TEST=y
Before merging this patch we also have to fix the lib_test_dynamic_uuid which fails on 32-bit.
Is it the test, or is it the UUID changes? I didn't go so far as to revert the UUID changes when I hit this on Pi 3 32b, just poked Caleb on irc.
While on 64-bit the expected GUID is generated (both on sandbox and qemu-riscv64_smode_defconfig), the generated GUID on 32-bit does not match the expected value. The generated GUID should be the same irrespective of the system.
Once we have fixed 32-bit little endian, we should start testing big endian.
Best regards
Heinrich
On Sun, Nov 03, 2024 at 04:56:40PM +0100, Heinrich Schuchardt wrote:
On 11/3/24 16:50, Tom Rini wrote:
On Sun, Nov 03, 2024 at 04:45:41PM +0100, Heinrich Schuchardt wrote:
On 11/3/24 08:12, Heinrich Schuchardt wrote:
The lib_test_uuid_to_le test fails on 32-bit systems. But we never caught this in our CI because we never ran any of our C unit tests on 32-bit.
Enable CONFIG_UNIT_TEST on qemu_arm_defconfig.
Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com
configs/qemu_arm_defconfig | 1 + 1 file changed, 1 insertion(+)
diff --git a/configs/qemu_arm_defconfig b/configs/qemu_arm_defconfig index d042aea49bb..cc4f4540fd5 100644 --- a/configs/qemu_arm_defconfig +++ b/configs/qemu_arm_defconfig @@ -67,3 +67,4 @@ CONFIG_TPM2_MMIO=y CONFIG_USB_EHCI_HCD=y CONFIG_USB_EHCI_PCI=y CONFIG_TPM=y +CONFIG_UNIT_TEST=y
Before merging this patch we also have to fix the lib_test_dynamic_uuid which fails on 32-bit.
Is it the test, or is it the UUID changes? I didn't go so far as to revert the UUID changes when I hit this on Pi 3 32b, just poked Caleb on irc.
While on 64-bit the expected GUID is generated (both on sandbox and qemu-riscv64_smode_defconfig), the generated GUID on 32-bit does not match the expected value. The generated GUID should be the same irrespective of the system.
Once we have fixed 32-bit little endian, we should start testing big endian.
Right. What I mean is, did this work prior to Caleb's UUID series that was merged with: commit 35394e1ea77ba0ad971d9115bd965a2403c0e031 Merge: 9eb0d731d800 7de51622a2cf Author: Tom Rini trini@konsulko.com Date: Fri Sep 13 08:20:25 2024 -0600
Merge tag 'efi-next-20241024' of https://source.denx.de/u-boot/custodians/u-boot-efi into n ext
Or has it always been wrong? To me, Patrick's report implies that it used to work.
On 11/3/24 17:12, Tom Rini wrote:
On Sun, Nov 03, 2024 at 04:56:40PM +0100, Heinrich Schuchardt wrote:
On 11/3/24 16:50, Tom Rini wrote:
On Sun, Nov 03, 2024 at 04:45:41PM +0100, Heinrich Schuchardt wrote:
On 11/3/24 08:12, Heinrich Schuchardt wrote:
The lib_test_uuid_to_le test fails on 32-bit systems. But we never caught this in our CI because we never ran any of our C unit tests on 32-bit.
Enable CONFIG_UNIT_TEST on qemu_arm_defconfig.
Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com
configs/qemu_arm_defconfig | 1 + 1 file changed, 1 insertion(+)diff --git a/configs/qemu_arm_defconfig b/configs/qemu_arm_defconfig index d042aea49bb..cc4f4540fd5 100644 --- a/configs/qemu_arm_defconfig +++ b/configs/qemu_arm_defconfig @@ -67,3 +67,4 @@ CONFIG_TPM2_MMIO=y CONFIG_USB_EHCI_HCD=y CONFIG_USB_EHCI_PCI=y CONFIG_TPM=y +CONFIG_UNIT_TEST=y
Before merging this patch we also have to fix the lib_test_dynamic_uuid which fails on 32-bit.
Is it the test, or is it the UUID changes? I didn't go so far as to revert the UUID changes when I hit this on Pi 3 32b, just poked Caleb on irc.
While on 64-bit the expected GUID is generated (both on sandbox and qemu-riscv64_smode_defconfig), the generated GUID on 32-bit does not match the expected value. The generated GUID should be the same irrespective of the system.
Once we have fixed 32-bit little endian, we should start testing big endian.
Right. What I mean is, did this work prior to Caleb's UUID series that was merged with: commit 35394e1ea77ba0ad971d9115bd965a2403c0e031 Merge: 9eb0d731d800 7de51622a2cf Author: Tom Rini trini@konsulko.com Date: Fri Sep 13 08:20:25 2024 -0600
Merge tag 'efi-next-20241024' of https://source.denx.de/u-boot/custodians/u-boot-efi into next
Or has it always been wrong? To me, Patrick's report implies that it used to work.
The functionality and the failing test were added by the series.
Best regards
Heinrich
On Sun, Nov 03, 2024 at 08:12:00AM +0100, Heinrich Schuchardt wrote:
The lib_test_uuid_to_le test fails on 32-bit systems. But we never caught this in our CI because we never ran any of our C unit tests on 32-bit.
Enable CONFIG_UNIT_TEST on qemu_arm_defconfig.
Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com
Reviewed-by: Tom Rini trini@konsulko.com
And it would be good to follow-up and turn it on for all QEMU defconfigs, I think at this point we should be able to. And if not, that's a few more unit tests to adjust.
participants (3)
-
Heinrich Schuchardt -
Ilias Apalodimas -
Tom Rini