[PATCH 2/2] fwts: uefi: clean up all test variable before doing the uefirtvariable test (LP: #1313554)

[IvanHu]

On 05/02/2014 05:44 PM, Colin Ian King wrote:
> On 28/04/14 09:07, Ivan Hu wrote:
>> Try to delete all the test variables that used by the uefirtvariable test,
>> before doing the uefirtvariable tests. Make sure there are no preexisting
>> test variables that cause the the uefirtvariable test fail, when the
>> variable tests accidentally stopped, terminated by the buggy firmware, power
>> fail etc.
>>
>> Signed-off-by: Ivan Hu <ivan.hu at canonical.com>
>> ---
>>   src/uefi/uefirtvariable/uefirtvariable.c |   31 ++++++++++++++++++++++++++++--
>>   1 file changed, 29 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/uefi/uefirtvariable/uefirtvariable.c b/src/uefi/uefirtvariable/uefirtvariable.c
>> index 6e40542..7ddfeb1 100644
>> --- a/src/uefi/uefirtvariable/uefirtvariable.c
>> +++ b/src/uefi/uefirtvariable/uefirtvariable.c
>> @@ -59,6 +59,33 @@ static uint32_t attributes =
>>   	FWTS_UEFI_VAR_BOOTSERVICE_ACCESS |
>>   	FWTS_UEFI_VAR_RUNTIME_ACCESS;
>>   static uint16_t variablenametest[] = {'T', 'e', 's', 't', 'v', 'a', 'r', '\0'};
>> +static uint16_t variablenametest2[] = {'T', 'e', 's', 't', 'v', 'a', 'r', ' ', '\0'};
>> +static uint16_t variablenametest3[] = {'T', 'e', 's', 't', 'v', 'a', '\0'};
>> +
>> +static void uefirtvariable_env_cleanup(void)
>> +{
>> +	struct efi_setvariable setvariable;
>> +	uint64_t status;
>> +	uint8_t data = 0;
>> +
>> +	setvariable.VariableName = variablenametest;
>> +	setvariable.VendorGuid = &gtestguid1;
>> +	setvariable.Attributes = 0;
>> +	setvariable.DataSize = 0;
>> +	setvariable.Data = &data;
>> +	setvariable.status = &status;
>> +	ioctl(fd, EFI_RUNTIME_SET_VARIABLE, &setvariable);
>> +
>> +	setvariable.VariableName = variablenametest2;
>> +	ioctl(fd, EFI_RUNTIME_SET_VARIABLE, &setvariable);
>> +
>> +	setvariable.VariableName = variablenametest3;
>> +	ioctl(fd, EFI_RUNTIME_SET_VARIABLE, &setvariable);
>> +
>> +	setvariable.VariableName = variablenametest;
>> +	setvariable.VendorGuid = &gtestguid2;
>> +	ioctl(fd, EFI_RUNTIME_SET_VARIABLE, &setvariable);
>> +}
>
> I see that we're not checking the ioctl return here.  What is the
> outcome if the variables do exist and the deletion fails to the tests
> that require these variables to not exist?

Actually, I expect that the test variables do not exist, I used the 
unique GUID, and name, and after each tests the test variables will be 
deleted. I delete the test variables without checking the result, 
because I don't care if the variables are deleted or not found. Adding 
the deleting code just for double confirm that the test variables do not 
exist. Because like the bug#1310686, back-to-back runs has different 
results issue,
https://bugs.launchpad.net/fwts/+bug/1310686
the buggy firmware set variable successfully with invalid attribute, but 
cannot be found by get variable, so that test variable doesn't be 
deleted. Although, another patch "uefi: uefirtvariable: report failure 
when setting variable with invalid attribute successfully" fixed this 
issue. But the machine has polluted, the test variable existed in that 
machine, it cannot be tested any more before manually delete the test 
variables, setvariable will return EFI_INVALID_PARAMETER. So I think it 
might be good idea to try to delete the test variable each time we would 
like to do the variable test.

>
> Perhaps a more "robust" thing to do is:
>
> Check if they exist, *then* attempt to deleted them, and if that fails,
> report this as an error as this will mess up subsequent tests.

Like the above example, the test variable do exist, but it cannot be 
found by getvariable on runtime. So it will not be deleted, and the 
problem is still there.

>
> Or am I being overly pedantic here?



>
>>
>>   static int uefirtvariable_init(fwts_framework *fw)
>>   {
>> @@ -78,6 +105,8 @@ static int uefirtvariable_init(fwts_framework *fw)
>>   		return FWTS_ABORTED;
>>   	}
>>
>> +	uefirtvariable_env_cleanup();
>> +
>>   	return FWTS_OK;
>>   }
>>
>> @@ -1044,8 +1073,6 @@ static int setvariable_test3(fwts_framework *fw)
>>   	int ret;
>>   	uint64_t datasize = 10;
>>   	uint8_t datadiff1 = 0, datadiff2 = 1, datadiff3 = 2;
>> -	uint16_t variablenametest2[] = {'T', 'e', 's', 't', 'v', 'a', 'r', ' ', '\0'};
>> -	uint16_t variablenametest3[] = {'T', 'e', 's', 't', 'v', 'a', '\0'};
>>
>>   	ret = setvariable_insertvariable(fw, attributes, datasize,
>>   		variablenametest2, &gtestguid1, datadiff2);
>>
>
>