Re: [Xen-devel] [PATCH v2 0.5/8] libx86: Proactively initialise error pointers

[Jan Beulich <jbeulich@xxxxxxxx>]

On 12.09.2019 20:55, Andrew Cooper wrote:
> This results in better behaviour for the caller.
> 
> Suggested-by: Jan Beulich <JBeulich@xxxxxxxx>
> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>

Acked-by: Jan Beulich <jbeulich@xxxxxxxx>

But I'm curious:

> --- a/tools/tests/cpu-policy/test-cpu-policy.c
> +++ b/tools/tests/cpu-policy/test-cpu-policy.c
> @@ -283,7 +283,7 @@ static void test_cpuid_deserialise_failure(void)
>      for ( size_t i = 0; i < ARRAY_SIZE(tests); ++i )
>      {
>          const struct test *t = &tests[i];
> -        uint32_t err_leaf = ~0u, err_subleaf = ~0u;
> +        uint32_t err_leaf, err_subleaf;
>          int rc;
>  
>          /* No writes should occur.  Use NULL to catch errors. */
> @@ -336,7 +336,7 @@ static void test_msr_deserialise_failure(void)
>      for ( size_t i = 0; i < ARRAY_SIZE(tests); ++i )
>      {
>          const struct test *t = &tests[i];
> -        uint32_t err_msr = ~0u;
> +        uint32_t err_msr;

Any reason for the (benign) switch from ~0u ...

> --- a/xen/lib/x86/cpuid.c
> +++ b/xen/lib/x86/cpuid.c
> @@ -381,6 +381,11 @@ int x86_cpuid_copy_from_buffer(struct cpuid_policy *p,
>      unsigned int i;
>      xen_cpuid_leaf_t data;
>  
> +    if ( err_leaf )
> +        *err_leaf = -1;
> +    if ( err_subleaf )
> +        *err_subleaf = -1;
> +
>      /*
>       * A well formed caller is expected to pass an array with leaves in 
> order,
>       * and without any repetitions.  However, due to per-vendor differences,
> --- a/xen/lib/x86/msr.c
> +++ b/xen/lib/x86/msr.c
> @@ -55,6 +55,9 @@ int x86_msr_copy_from_buffer(struct msr_policy *p,
>      xen_msr_entry_t data;
>      int rc;
>  
> +    if ( err_msr )
> +        *err_msr = -1;

... to -1?

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel