Re: [Xen-devel] [PATCH] build/printf: fix incorrect format specifiers

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 09.02.17 at 17:35, <roger.pau@xxxxxxxxxx> wrote:
> --- a/xen/arch/x86/cpu/mcheck/mce.c
> +++ b/xen/arch/x86/cpu/mcheck/mce.c
> @@ -596,8 +596,8 @@ int show_mca_info(int inited, struct cpuinfo_x86 *c)
>          };
>  
>          snprintf(prefix, ARRAY_SIZE(prefix),
> -                 g_type != mcheck_unset ? XENLOG_WARNING "CPU%i: "
> -                 : XENLOG_INFO,
> +                 g_type != mcheck_unset ? XENLOG_WARNING "CPU%i: ":
> +                                          XENLOG_INFO "CPU%i: ",

At the very least there a blank missing ahead of the colon. But I
think we generally prefer to align the colon with the question
mark, despite otherwise placing operators last on a line when
needing to break it. Plus I don#t see why you want the format
string duplicated - just use

        snprintf(prefix, ARRAY_SIZE(prefix), "%sCPU%i: ",
                 g_type != mcheck_unset ? XENLOG_WARNING : XENLOG_INFO,
                 smp_processor_id());

> --- a/xen/common/grant_table.c
> +++ b/xen/common/grant_table.c
> @@ -3284,7 +3284,7 @@ gnttab_release_mappings(
>  
>          ref = map->ref;
>  
> -        gdprintk(XENLOG_INFO, "Grant release (%hu) ref:(%hu) "
> +        gdprintk(XENLOG_INFO, "Grant release (%u) ref:(%u) "
>                  "flags:(%x) dom:(%hu)\n",

I have always been puzzled by these h modifiers; I don't think it's
useful to have even for the domain ID (which after all we print
with %d almost everywhere else).

Since you're touching these anyway, I'd like to also bring up the
question of decimal vs hex: The larger the amount of grants in use,
the less useful I consider decimal numbers being logged. So perhaps
these should switch to %#x at once.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel