On 07/11/2023 10:33 am, Nicola Vetrini wrote:
> The backwards goto in the vsnprintf function can be replaced
> with a loop, thereby fixing a violation of MISRA C:2012 Rule 15.2.
>
> Signed-off-by: Nicola Vetrini <nicola.vetr...@bugseng.com>
> ---
> xen/common/vsprintf.c | 20 ++++++++++++--------
> 1 file changed, 12 insertions(+), 8 deletions(-)
>
> diff --git a/xen/common/vsprintf.c b/xen/common/vsprintf.c
> index c49631c0a4d8..603bae44177a 100644
> --- a/xen/common/vsprintf.c
> +++ b/xen/common/vsprintf.c
> @@ -495,6 +495,8 @@ int vsnprintf(char *buf, size_t size, const char *fmt,
> va_list args)
> }
>
> for (; *fmt ; ++fmt) {
> + bool repeat = true;
> +
> if (*fmt != '%') {
> if (str < end)
> *str = *fmt;
> @@ -504,14 +506,16 @@ int vsnprintf(char *buf, size_t size, const char *fmt,
> va_list args)
>
> /* process flags */
> flags = 0;
> - repeat:
> - ++fmt; /* this also skips first '%' */
> - switch (*fmt) {
> - case '-': flags |= LEFT; goto repeat;
> - case '+': flags |= PLUS; goto repeat;
> - case ' ': flags |= SPACE; goto repeat;
> - case '#': flags |= SPECIAL; goto repeat;
> - case '0': flags |= ZEROPAD; goto repeat;
> + while ( repeat ) {
> + ++fmt; /* this also skips the first '%' */
> + switch (*fmt) {
> + case '-': flags |= LEFT; break;
> + case '+': flags |= PLUS; break;
> + case ' ': flags |= SPACE; break;
> + case '#': flags |= SPECIAL; break;
> + case '0': flags |= ZEROPAD; break;
> + default: repeat = false; break;
> + }
I'm firmly against this change. It takes a simple and clear piece of
code and replaces it with something harder to follow because you have to
look elsewhere to figure how the variable works.
Labels with names such as repeat/again/retry are clearly forming a
loop(ish).
I see in patch 4 that you exempt again/retry. That list needs to
include repeat, and this patch wants dropping.
~Andrew
