| Message ID | b1f2dc6467571090f882ce7c0611db13a8c63555.1712042178.git.federico.serafini@bugseng.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | xen: address violations of MISRA C:2012 Rule 16.3 | expand |
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.
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;
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(-)