Re: [PATCH] unit: check cipher support and cleanup in test-storage

[James Prestwood <prestwoj@gmail.com>]

Hi,

On 5/5/25 5:39 AM, Marcel Holtmann wrote:
> Hi James,
>
>> The __storage_decrypt API requires AES_CTR so support should be
>> checked before running that test. In addition storage_exit was never
>> being called which leaves unbalanced mlock/munlock calls.
>> ---
>> unit/test-storage.c | 11 +++++++++--
>> 1 file changed, 9 insertions(+), 2 deletions(-)
>>
>> diff --git a/unit/test-storage.c b/unit/test-storage.c
>> index c40518e6..765d9967 100644
>> --- a/unit/test-storage.c
>> +++ b/unit/test-storage.c
>> @@ -44,12 +44,19 @@ static void test_short_encrypted_bytes(const void *data)
>>
>> int main(int argc, char *argv[])
>> {
>> + int ret;
>> +
>> l_test_init(&argc, &argv);
>>
>> storage_init((const uint8_t *)"abc123", 6);
>>
>> - l_test_add("/storage/profile encryption",
>> + if (l_cipher_is_supported(L_CIPHER_AES_CTR))
>> + l_test_add("/storage/profile encryption",
>> test_short_encrypted_bytes, NULL);
> just use L_TEST_FLAG_ALLOW_FAILURE instead.
Ok, new way of doing things I guess.
>
>> - return l_test_run();
>> + ret = l_test_run();
>> +
>> + storage_exit();
>> +
>> + return ret;
> I really prefer we keep “return l_test_run()” as the basics on how test case are run.
>
> Just put storage_init,storage_exit into the test case itself. I would just do system_key_set = false in the exit function.

I'm fine with this, but its actually not possible to perform the cleanup 
after a failed test. If we want to do this we need actual cleanup 
support in l_test. Looks like we'd need to add a destroy function to 
l_test_add_data_func().

Does that sound ok?

Thanks,

James

>
> Regards
>
> Marcel
>