diff mbox series

[RFC,2/4] x86/dom0: make goto jump forward

Message ID fcc14cb3859e4904f4a17e14fc41b5e05ae1f05f.1699295113.git.nicola.vetrini@bugseng.com (mailing list archive)
State New, archived
Headers show
Series address MISRA C:2012 Rule 15.2 | expand

Commit Message

Nicola Vetrini Nov. 7, 2023, 10:33 a.m. UTC
The jump to the label 'parse_error' becomes forward, rather
than backward; at the same time, the else branch can be eliminated.

This also fixes a violation of MISRA C:2012 Rule 15.2.

Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
---
 xen/arch/x86/dom0_build.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

Comments

Jan Beulich Nov. 21, 2023, 1:57 p.m. UTC | #1
On 07.11.2023 11:33, Nicola Vetrini wrote:
> The jump to the label 'parse_error' becomes forward, rather
> than backward; at the same time, the else branch can be eliminated.

What "else branch" is this referring to?

> This also fixes a violation of MISRA C:2012 Rule 15.2.
> 
> Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
> ---
>  xen/arch/x86/dom0_build.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/xen/arch/x86/dom0_build.c b/xen/arch/x86/dom0_build.c
> index 09fb8b063ae7..f0191dc148a2 100644
> --- a/xen/arch/x86/dom0_build.c
> +++ b/xen/arch/x86/dom0_build.c
> @@ -439,12 +439,7 @@ static void __init process_dom0_ioports_disable(struct domain *dom0)
>      {
>          io_from = simple_strtoul(t, &u, 16);
>          if ( u == t )
> -        {
> -        parse_error:
> -            printk("Invalid ioport range <%s> "
> -                   "in dom0_ioports_disable, skipping\n", t);
> -            continue;
> -        }
> +            goto parse_error;
>  
>          if ( *u == '\0' )
>              io_to = io_from;
> @@ -454,7 +449,12 @@ static void __init process_dom0_ioports_disable(struct domain *dom0)
>              goto parse_error;
>  
>          if ( (*u != '\0') || (io_to < io_from) || (io_to >= 65536) )
> -            goto parse_error;
> +        {
> +        parse_error:
> +            printk("Invalid ioport range <%s> "
> +                   "in dom0_ioports_disable, skipping\n", t);
> +            continue;
> +        }
>  
>          printk("Disabling dom0 access to ioport range %04lx-%04lx\n",
>              io_from, io_to);

While purely from how the change looks I think I could live with this kind
of code adjustment (not a lot of churn, in particular no re-indentation,
no potentially harder to follow use of [new] local variables), this is
precisely one of the cases where I think having the label appear before
the goto is actually better, for matching how the language works otherwise
(things you want to use first need to be declared / defined).

Jan
diff mbox series

Patch

diff --git a/xen/arch/x86/dom0_build.c b/xen/arch/x86/dom0_build.c
index 09fb8b063ae7..f0191dc148a2 100644
--- a/xen/arch/x86/dom0_build.c
+++ b/xen/arch/x86/dom0_build.c
@@ -439,12 +439,7 @@  static void __init process_dom0_ioports_disable(struct domain *dom0)
     {
         io_from = simple_strtoul(t, &u, 16);
         if ( u == t )
-        {
-        parse_error:
-            printk("Invalid ioport range <%s> "
-                   "in dom0_ioports_disable, skipping\n", t);
-            continue;
-        }
+            goto parse_error;
 
         if ( *u == '\0' )
             io_to = io_from;
@@ -454,7 +449,12 @@  static void __init process_dom0_ioports_disable(struct domain *dom0)
             goto parse_error;
 
         if ( (*u != '\0') || (io_to < io_from) || (io_to >= 65536) )
-            goto parse_error;
+        {
+        parse_error:
+            printk("Invalid ioport range <%s> "
+                   "in dom0_ioports_disable, skipping\n", t);
+            continue;
+        }
 
         printk("Disabling dom0 access to ioport range %04lx-%04lx\n",
             io_from, io_to);