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 > 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
