Hi Simon,
Le 12/06/2026 à 20:32, Simon Glass a écrit :
This Mail comes from Outside of SoftAtHome: Do not answer, click links or open
attachments unless you recognize the sender and know the content is safe.
Hi Philippe,
On 2026-06-01T13:42:58, Philippe Reynes <[email protected]> wrote:
test: dm: ecdsa.c: clean this test as software ecdsa is now implemented
The test ecdsa was done when ecdsa was only supported by hardware.
So it wasn't possible to test ecdsa on sandbox, and there is a test
to check that ecdsa is not supported on sandbox.
Now, there is a software implementation of ecdsa. So we add a test
to verify that ecdsa_verify may be used on sandbox.
Signed-off-by: Philippe Reynes <[email protected]>
Reviewed-by: Raymond Mao <[email protected]>
test/dm/ecdsa.c | 107 ++++++++++++++++++++++++++++++++++++++++++++++++++------
1 file changed, 97 insertions(+), 10 deletions(-)
Reviewed-by: Simon Glass <[email protected]>
with nits below
Thanks a lot for this feedback.
Do you prefer that I send a v9 or is this v8 acceptable please ?
Regards,
Philippe
diff --git a/test/dm/ecdsa.c b/test/dm/ecdsa.c
@@ -3,36 +3,123 @@
+ ret = fdt_property(fdt, name, value, len);
+ if (ret)
+ goto out;
+
+out:
+ free(value);
+ return ret;
The 'if (ret) goto out' jumps to the very next statement, so it does
nothing - please drop it and fall through to the label. Also, since
this is test code and all callers pass valid pointers, the NULL checks
on fdt, name and data at the top of this helper are not really needed.
diff --git a/test/dm/ecdsa.c b/test/dm/ecdsa.c
@@ -3,36 +3,123 @@
+ /* prepare the signature */
+ ut_assertok(hex2bin(sig + 0, r, strlen(r) / 2));
+ ut_assertok(hex2bin(sig + 32, s, strlen(s) / 2));
The 32 here hard-codes the size of r, while you already compute
strlen(r) / 2 on the same line - please use that (or a named constant)
for the offset, so the two cannot get out of sync.
diff --git a/test/dm/ecdsa.c b/test/dm/ecdsa.c
@@ -3,36 +3,123 @@
ut_assertok(uclass_get(UCLASS_ECDSA, &ucp));
ut_assertnonnull(ucp);
- ut_asserteq(-ENODEV, ecdsa_verify(&info, NULL, 0, NULL, 0));
+ ut_assertok(ecdsa_verify(&info, region, 1, sig, sizeof(sig)));
Can you drop ucp now?
Also, the subject says 'clean this test' but the patch actually
extends it to do a real verification against an RFC 6979 vector.
Please can you reword it to describe that, and use 'test: dm: ecdsa:'
rather than 'ecdsa.c:' as the tag, matching the usual style?
Regards,
Simon