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

[Federico Serafini]

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)