MISRA C:2012 Rule 16.3 states: "An unconditional `break' statement
shall terminate every switch-clause".

Add break statement to address violations of the rule or add
pseudo-keyword fallthrough to meet the requirements to deviate it.

No functional change.

Signed-off-by: Federico Serafini <federico.serafini@bugseng.com>
---
 xen/common/vsprintf.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/xen/common/vsprintf.c b/xen/common/vsprintf.c
index c49631c0a4..612751c90f 100644
--- a/xen/common/vsprintf.c
+++ b/xen/common/vsprintf.c
@@ -377,7 +377,7 @@ static char *pointer(char *str, const char *end, const char **fmt_ptr,
             str = number(str, end, hex_buffer[i], 16, 2, -1, ZEROPAD);
 
             if ( ++i == field_width )
-                return str;
+                break;
 
             if ( sep )
             {
@@ -386,6 +386,8 @@ static char *pointer(char *str, const char *end, const char **fmt_ptr,
                 ++str;
             }
         }
+
+        return str;
     }
 
     case 'p': /* PCI SBDF. */
@@ -619,6 +621,7 @@ int vsnprintf(char *buf, size_t size, const char *fmt, va_list args)
 
         case 'X':
             flags |= LARGE;
+            fallthrough;
         case 'x':
             base = 16;
             break;
@@ -626,6 +629,7 @@ int vsnprintf(char *buf, size_t size, const char *fmt, va_list args)
         case 'd':
         case 'i':
             flags |= SIGN;
+            fallthrough;
         case 'u':
             break;
 
-- 
2.34.1

On 02.04.2024 09:22, Federico Serafini wrote:
> MISRA C:2012 Rule 16.3 states: "An unconditional `break' statement
> shall terminate every switch-clause".
> 
> Add break statement to address violations of the rule or add
> pseudo-keyword fallthrough to meet the requirements to deviate it.

While the latter half of the sentence properly describes the latter
two of the hunks, the former half doesn't match the former two hunks
at all:

> --- a/xen/common/vsprintf.c
> +++ b/xen/common/vsprintf.c
> @@ -377,7 +377,7 @@ static char *pointer(char *str, const char *end, const char **fmt_ptr,
>              str = number(str, end, hex_buffer[i], 16, 2, -1, ZEROPAD);
>  
>              if ( ++i == field_width )
> -                return str;
> +                break;

This "break" is inside for(), not switch().

> @@ -386,6 +386,8 @@ static char *pointer(char *str, const char *end, const char **fmt_ptr,
>                  ++str;
>              }
>          }
> +
> +        return str;
>      }

And this "return" is what now "delimits" case 'h' of the switch(). The
original situation therefore was that the for() could not be exited by
other than the "return" inside. The supposedly missing "break" in that
arrangement would have been "unreachable code", i.e. violate another
rule. Hence the (undescribed) further re-arrangement.

Jan

On 03/04/24 09:06, Jan Beulich wrote:
> On 02.04.2024 09:22, Federico Serafini wrote:
>> MISRA C:2012 Rule 16.3 states: "An unconditional `break' statement
>> shall terminate every switch-clause".
>>
>> Add break statement to address violations of the rule or add
>> pseudo-keyword fallthrough to meet the requirements to deviate it.
> 
> While the latter half of the sentence properly describes the latter
> two of the hunks, the former half doesn't match the former two hunks
> at all:
> 
>> --- a/xen/common/vsprintf.c
>> +++ b/xen/common/vsprintf.c
>> @@ -377,7 +377,7 @@ static char *pointer(char *str, const char *end, const char **fmt_ptr,
>>               str = number(str, end, hex_buffer[i], 16, 2, -1, ZEROPAD);
>>   
>>               if ( ++i == field_width )
>> -                return str;
>> +                break;
> 
> This "break" is inside for(), not switch().
> 
>> @@ -386,6 +386,8 @@ static char *pointer(char *str, const char *end, const char **fmt_ptr,
>>                   ++str;
>>               }
>>           }
>> +
>> +        return str;
>>       }
> 
> And this "return" is what now "delimits" case 'h' of the switch(). The
> original situation therefore was that the for() could not be exited by
> other than the "return" inside. The supposedly missing "break" in that
> arrangement would have been "unreachable code", i.e. violate another
> rule. Hence the (undescribed) further re-arrangement.

I'll improve the description.

-- 
Federico Serafini, M.Sc.

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