[PATCH v8 06/15] test: dm: ecdsa.c: clean this test as software ecdsa is now implemented

[Philippe Reynes]

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 <philippe.reynes at softathome.com> 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 <philippe.reynes at softathome.com>
>> Reviewed-by: Raymond Mao <raymondmaoca at gmail.com>
>>
>> test/dm/ecdsa.c | 107 ++++++++++++++++++++++++++++++++++++++++++++++++++------
>>   1 file changed, 97 insertions(+), 10 deletions(-)
> Reviewed-by: Simon Glass <sjg at chromium.org>
>
> 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