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

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?

we can add that, but I don’t think it is needed. Every test case is run in its own process. And since you only have one test case at the moment, don’t over engineer this.

Regards

Marcel