[PATCH] uefirtvariable: check the return value for cleaning up the enviroment

[Alex Hung]

On 12/03/2014 11:20 AM, Ivan Hu wrote:
> Coverity Scan complains the Unchecked return value for ioctl() calls in
> uefirtvariable_env_cleanup(). The uefirtvariable_env_cleanup() is added to avoid
> some buggy firmware abnormal stop the tests without deleted the test variables.
> Before starting the test, it will try to delete all the test variables. Expected
> return -1 and errno EINVAL when no test variables exist, return 0 when there are
> test variables and deleted sucessfully. Check the return value and log info
> when other errors occur.
> 
> Signed-off-by: Ivan Hu <ivan.hu at canonical.com>
> ---
>  src/uefi/uefirtvariable/uefirtvariable.c |   26 ++++++++++++++++++++------
>  1 file changed, 20 insertions(+), 6 deletions(-)
> 
> diff --git a/src/uefi/uefirtvariable/uefirtvariable.c b/src/uefi/uefirtvariable/uefirtvariable.c
> index a19f835..3bb5ca0 100644
> --- a/src/uefi/uefirtvariable/uefirtvariable.c
> +++ b/src/uefi/uefirtvariable/uefirtvariable.c
> @@ -21,6 +21,7 @@
>  #include <stdio.h>
>  #include <stddef.h>
>  #include <sys/ioctl.h>
> +#include <errno.h>
>  #include <fcntl.h>
>  
>  #include "fwts.h"
> @@ -62,8 +63,9 @@ 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)
> +static void uefirtvariable_env_cleanup(fwts_framework *fw)
>  {
> +	long ioret;
>  	struct efi_setvariable setvariable;
>  	uint64_t status;
>  	uint8_t data = 0;
> @@ -74,17 +76,29 @@ static void uefirtvariable_env_cleanup(void)
>  	setvariable.DataSize = 0;
>  	setvariable.Data = &data;
>  	setvariable.status = &status;
> -	ioctl(fd, EFI_RUNTIME_SET_VARIABLE, &setvariable);
> +	ioret = ioctl(fd, EFI_RUNTIME_SET_VARIABLE, &setvariable);
> +	if (ioret == -1 && errno != EINVAL)
> +		fwts_log_info(fw, "Got errors when clean up the enviroment, "
> +					"but still try to run the tests.");
>  
>  	setvariable.VariableName = variablenametest2;
> -	ioctl(fd, EFI_RUNTIME_SET_VARIABLE, &setvariable);
> +	ioret = ioctl(fd, EFI_RUNTIME_SET_VARIABLE, &setvariable);
> +	if (ioret == -1 && errno != EINVAL)
> +		fwts_log_info(fw, "Got errors when clean up the enviroment, "
> +					"but still try to run the tests.");
>  
>  	setvariable.VariableName = variablenametest3;
> -	ioctl(fd, EFI_RUNTIME_SET_VARIABLE, &setvariable);
> +	ioret = ioctl(fd, EFI_RUNTIME_SET_VARIABLE, &setvariable);
> +	if (ioret == -1 && errno != EINVAL)
> +		fwts_log_info(fw, "Got errors when clean up the enviroment, "
> +					"but still try to run the tests.");
>  
>  	setvariable.VariableName = variablenametest;
>  	setvariable.VendorGuid = &gtestguid2;
> -	ioctl(fd, EFI_RUNTIME_SET_VARIABLE, &setvariable);
> +	ioret = ioctl(fd, EFI_RUNTIME_SET_VARIABLE, &setvariable);
> +	if (ioret == -1 && errno != EINVAL)
> +		fwts_log_info(fw, "Got errors when clean up the enviroment, "
> +					"but still try to run the tests.");
>  }
>  
>  static int uefirtvariable_init(fwts_framework *fw)
> @@ -105,7 +119,7 @@ static int uefirtvariable_init(fwts_framework *fw)
>  		return FWTS_ABORTED;
>  	}
>  
> -	uefirtvariable_env_cleanup();
> +	uefirtvariable_env_cleanup(fw);
>  
>  	return FWTS_OK;
>  }
> 

Since errors are returned, will it be better if warnings are used instead?

-- 
Cheers,
Alex Hung