[PATCH][RESEND] UEFI: move the UEFI tests into the UEFI categroy (LP:#1207200)

[Colin Ian King]

On 02/08/13 05:41, Ivan Hu wrote:
> From: IvanHu <ivan.hu at canonical.com>
>
> Moving the uefirtvariable, uefirttime and uefirtmisc UEFI runtime service
> tests in the the UEFI category instead of using unsafe category in order
> to make it more specific and let users to be more aware of the tests.
>
> Signed-off-by: Ivan Hu <ivan.hu at canonical.com>
> ---
>   src/lib/src/fwts_framework.c             |   10 ++++++++--
>   src/uefi/uefirtmisc/uefirtmisc.c         |    2 +-
>   src/uefi/uefirttime/uefirttime.c         |    2 +-
>   src/uefi/uefirtvariable/uefirtvariable.c |    2 +-
>   4 files changed, 11 insertions(+), 5 deletions(-)
>
> diff --git a/src/lib/src/fwts_framework.c b/src/lib/src/fwts_framework.c
> index 1581681..28c2ce8 100644
> --- a/src/lib/src/fwts_framework.c
> +++ b/src/lib/src/fwts_framework.c
> @@ -38,7 +38,8 @@
>   	 FWTS_FLAG_INTERACTIVE_EXPERIMENTAL |	\
>   	 FWTS_FLAG_POWER_STATES |		\
>   	 FWTS_FLAG_UTILS |			\
> -	 FWTS_FLAG_UNSAFE)
> +	 FWTS_FLAG_UNSAFE |			\
> +	 FWTS_FLAG_TEST_UEFI)
>
>   static fwts_list tests_to_skip;
>
> @@ -81,6 +82,7 @@ static fwts_option fwts_framework_options[] = {
>   	{ "filter-error-keep",	"",   1, "Keep errors that match any of the specified labels." },
>   	{ "acpica-debug",	"",   0, "Enable ACPICA debug/warning messages." },
>   	{ "acpica",		"",   1, "Enable ACPICA run time options." },
> +	{ "uefi",		"",   0, "Run UEFI tests." },
>   	{ NULL, NULL, 0, NULL }
>   };
>
> @@ -227,6 +229,7 @@ static void fwts_framework_show_tests(fwts_framework *fw, const bool full)
>   		{ "Power States",		FWTS_FLAG_POWER_STATES },
>   		{ "Utilities",			FWTS_FLAG_UTILS },
>   		{ "Unsafe",			FWTS_FLAG_UNSAFE },
> +		{ "UEFI",			FWTS_FLAG_TEST_UEFI },
>   		{ NULL,				0 },
>   	};
>
> @@ -241,7 +244,7 @@ static void fwts_framework_show_tests(fwts_framework *fw, const bool full)
>   			fwts_list_init(&sorted);
>   			fwts_list_foreach(item, &fwts_framework_test_list) {
>   				test = fwts_list_data(fwts_framework_test *, item);
> -				if ((test->flags & FWTS_FLAG_RUN_ALL) == categories[i].flag)
> +				if ((test->flags & FWTS_FLAG_RUN_ALL) & categories[i].flag)
>   					fwts_list_add_ordered(&sorted, test,
>   						fwts_framework_compare_test_name);
>   			}
> @@ -1130,6 +1133,9 @@ int fwts_framework_options_handler(fwts_framework *fw, int argc, char * const ar
>   			if (fwts_framework_acpica_parse(fw, optarg) != FWTS_OK)
>   				return FWTS_ERROR;
>   			break;
> +		case 38: /* --uefi */
> +			fw->flags |= FWTS_FLAG_TEST_UEFI;
> +			break;
>   		}
>   		break;
>   	case 'a': /* --all */
> diff --git a/src/uefi/uefirtmisc/uefirtmisc.c b/src/uefi/uefirtmisc/uefirtmisc.c
> index 8fd4b4c..d94b885 100644
> --- a/src/uefi/uefirtmisc/uefirtmisc.c
> +++ b/src/uefi/uefirtmisc/uefirtmisc.c
> @@ -224,4 +224,4 @@ static fwts_framework_ops uefirtmisc_ops = {
>   	.minor_tests = uefirtmisc_tests
>   };
>
> -FWTS_REGISTER("uefirtmisc", &uefirtmisc_ops, FWTS_TEST_ANYTIME, FWTS_FLAG_UNSAFE | FWTS_FLAG_ROOT_PRIV);
> +FWTS_REGISTER("uefirtmisc", &uefirtmisc_ops, FWTS_TEST_ANYTIME, FWTS_FLAG_TEST_UEFI | FWTS_FLAG_UNSAFE | FWTS_FLAG_ROOT_PRIV);
> diff --git a/src/uefi/uefirttime/uefirttime.c b/src/uefi/uefirttime/uefirttime.c
> index 2094677..c02a1aa 100644
> --- a/src/uefi/uefirttime/uefirttime.c
> +++ b/src/uefi/uefirttime/uefirttime.c
> @@ -497,4 +497,4 @@ static fwts_framework_ops uefirttime_ops = {
>   	.minor_tests = uefirttime_tests
>   };
>
> -FWTS_REGISTER("uefirttime", &uefirttime_ops, FWTS_TEST_ANYTIME, FWTS_FLAG_UNSAFE | FWTS_FLAG_ROOT_PRIV);
> +FWTS_REGISTER("uefirttime", &uefirttime_ops, FWTS_TEST_ANYTIME, FWTS_FLAG_TEST_UEFI | FWTS_FLAG_UNSAFE | FWTS_FLAG_ROOT_PRIV);
> diff --git a/src/uefi/uefirtvariable/uefirtvariable.c b/src/uefi/uefirtvariable/uefirtvariable.c
> index e223f82..4425029 100644
> --- a/src/uefi/uefirtvariable/uefirtvariable.c
> +++ b/src/uefi/uefirtvariable/uefirtvariable.c
> @@ -1654,4 +1654,4 @@ static fwts_framework_ops uefirtvariable_ops = {
>   	.options_check   = options_check,
>   };
>
> -FWTS_REGISTER("uefirtvariable", &uefirtvariable_ops, FWTS_TEST_ANYTIME, FWTS_FLAG_UNSAFE | FWTS_FLAG_ROOT_PRIV);
> +FWTS_REGISTER("uefirtvariable", &uefirtvariable_ops, FWTS_TEST_ANYTIME, FWTS_FLAG_TEST_UEFI | FWTS_FLAG_UNSAFE | FWTS_FLAG_ROOT_PRIV);
>
This is more like it.  The changes are good.  My concern is that the 
potentially damaging "unsafe" tests are enabled for the --all option 
which may be a risk. But "--all" does mean all, so that is fair really I 
guess.

So, I'm happy with these changes as long as we believe the risk of 
enabling the "unsafe" tests with the --all option is low enough. 
Otherwise, perhaps we should remove unsafe tests from --all just to 
avoid potentially bricking users machines.  So anyone like to also 
comment on that?

Colin