Re: [XEN PATCH v2] xen/set_{c,p}x_pminfo: address violations od MISRA C:2012 Rule 8.3

[Jan Beulich <jbeulich@xxxxxxxx>]

On 31.10.2023 09:33, Federico Serafini wrote:
> Make function definitions and declarations consistent.
> No functional change.
> 
> Signed-off-by: Federico Serafini <federico.serafini@xxxxxxxxxxx>

Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>

> @@ -488,22 +488,22 @@ int set_px_pminfo(uint32_t acpi_id, struct 
> xen_processor_performance *dom0_px_in
>      pmpt->acpi_id = acpi_id;
>      pmpt->id = cpuid;
>  
> -    if ( dom0_px_info->flags & XEN_PX_PCT )
> +    if ( perf->flags & XEN_PX_PCT )
>      {
>          /* space_id check */
> -        if (dom0_px_info->control_register.space_id != 
> -            dom0_px_info->status_register.space_id)
> +        if ( perf->control_register.space_id !=
> +             perf->status_register.space_id )
>          {
>              ret = -EINVAL;
>              goto out;
>          }
>  
> -        memcpy ((void *)&pxpt->control_register,
> -                (void *)&dom0_px_info->control_register,
> -                sizeof(struct xen_pct_register));
> -        memcpy ((void *)&pxpt->status_register,
> -                (void *)&dom0_px_info->status_register,
> -                sizeof(struct xen_pct_register));
> +        memcpy((void *)&pxpt->control_register,
> +               (void *)&perf->control_register,
> +               sizeof(struct xen_pct_register));
> +        memcpy((void *)&pxpt->status_register,
> +               (void *)&perf->status_register,
> +               sizeof(struct xen_pct_register));

Along with the other coding style changes it might have been nice to
also drop the bogus casts here and ...

> @@ -512,69 +512,67 @@ int set_px_pminfo(uint32_t acpi_id, struct 
> xen_processor_performance *dom0_px_in
>          }
>      }
>  
> -    if ( dom0_px_info->flags & XEN_PX_PSS ) 
> +    if ( perf->flags & XEN_PX_PSS )
>      {
>          /* capability check */
> -        if (dom0_px_info->state_count <= 1)
> +        if ( perf->state_count <= 1 )
>          {
>              ret = -EINVAL;
>              goto out;
>          }
>  
>          if ( !(pxpt->states = xmalloc_array(struct xen_processor_px,
> -                        dom0_px_info->state_count)) )
> +                                            perf->state_count)) )
>          {
>              ret = -ENOMEM;
>              goto out;
>          }
> -        if ( copy_from_guest(pxpt->states, dom0_px_info->states,
> -                             dom0_px_info->state_count) )
> +        if ( copy_from_guest(pxpt->states, perf->states, perf->state_count) )
>          {
>              ret = -EFAULT;
>              goto out;
>          }
> -        pxpt->state_count = dom0_px_info->state_count;
> +        pxpt->state_count = perf->state_count;
>  
>          if ( cpufreq_verbose )
>              print_PSS(pxpt->states,pxpt->state_count);
>      }
>  
> -    if ( dom0_px_info->flags & XEN_PX_PSD )
> +    if ( perf->flags & XEN_PX_PSD )
>      {
>          /* check domain coordination */
> -        if (dom0_px_info->shared_type != CPUFREQ_SHARED_TYPE_ALL &&
> -            dom0_px_info->shared_type != CPUFREQ_SHARED_TYPE_ANY &&
> -            dom0_px_info->shared_type != CPUFREQ_SHARED_TYPE_HW)
> +        if ( perf->shared_type != CPUFREQ_SHARED_TYPE_ALL &&
> +             perf->shared_type != CPUFREQ_SHARED_TYPE_ANY &&
> +             perf->shared_type != CPUFREQ_SHARED_TYPE_HW )
>          {
>              ret = -EINVAL;
>              goto out;
>          }
>  
> -        pxpt->shared_type = dom0_px_info->shared_type;
> -        memcpy ((void *)&pxpt->domain_info,
> -                (void *)&dom0_px_info->domain_info,
> -                sizeof(struct xen_psd_package));
> +        pxpt->shared_type = perf->shared_type;
> +        memcpy((void *)&pxpt->domain_info,
> +               (void *)&perf->domain_info,
> +               sizeof(struct xen_psd_package));

... here. If I end up committing this, I may take the liberty to do so.

Jan