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

[Federico Serafini <federico.serafini@xxxxxxxxxxx>]

On 27/10/23 15:57, Jan Beulich wrote:
> On 26.10.2023 14:09, Federico Serafini wrote:
> > --- a/xen/drivers/cpufreq/cpufreq.c
> > +++ b/xen/drivers/cpufreq/cpufreq.c
> > @@ -457,14 +457,14 @@ static void print_PPC(unsigned int platform_limit)
> >       printk("\t_PPC: %d\n", platform_limit);
> >   }
> >   
> > -int set_px_pminfo(uint32_t acpi_id, struct xen_processor_performance *dom0_px_info)
> > +int set_px_pminfo(uint32_t acpi_id, struct xen_processor_performance *perf)
> >   {
> >       int ret=0, cpuid;
> >       struct processor_pminfo *pmpt;
> >       struct processor_performance *pxpt;
> >   
> >       cpuid = get_cpu_id(acpi_id);
> > -    if ( cpuid < 0 || !dom0_px_info)
> > +    if ( ( cpuid < 0 ) || !perf)
> I don't see the need for the new pair of parentheses here, but if you
> add such, please don't violate style.
Noted.

> > @@ -488,21 +488,21 @@ 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,
> > +                (void *)&perf->control_register,
> >                   sizeof(struct xen_pct_register));
> >           memcpy ((void *)&pxpt->status_register,
> > -                (void *)&dom0_px_info->status_register,
> > +                (void *)&perf->status_register,
> >                   sizeof(struct xen_pct_register));
> >   
> >           if ( cpufreq_verbose )
> > @@ -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)
> Since you're adjusting style elsewhere, would you mind adding the missing
> blanks here as well?
>
> >           {
> >               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)
> Same here then?
>
> Jan
Sure, I'll pay more attention.

--
Federico Serafini, M.Sc.

Software Engineer, BUGSENG (http://bugseng.com)