Re: [PATCH] tools/bootconfig: Fix wrong __VA_ARGS__ usage

[Masami Hiramatsu]

On Sat, 08 Feb 2020 22:10:40 +1100
Michael Ellerman <mpe@xxxxxxxxxxxxxx> wrote:

> Masami Hiramatsu <mhiramat@xxxxxxxxxx> writes:
> > Since printk() wrapper macro uses __VA_ARGS__ without
> > "##" prefix, it causes a build error if there is no
> > variable arguments (e.g. only fmt is specified.)
> > To fix this error, use ##__VA_ARGS__ instead of
> > __VAR_ARGS__.
> >
> > Fixes: 950313ebf79c ("tools: bootconfig: Add bootconfig command")
> > Reported-by: Michael Ellerman <mpe@xxxxxxxxxxxxxx>
> > Signed-off-by: Masami Hiramatsu <mhiramat@xxxxxxxxxx>
> > ---
> >  tools/bootconfig/include/linux/printk.h |    2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> Thanks that builds for me.
> 
> The output when adding to a fresh initrd is a bit confusing though, eg:
> 
>   $ ./bootconfig -a samples/good-simple.bconf initrd.img
>   Apply samples/good-simple.bconf to initrd.img
>           Number of nodes: 13
>           Size: 120 bytes
>           Checksum: 9036
>   checksum error: 0 != 444373994
>   $ echo $?
>   0
> 
> ie. the checksum error.

Hmm...

> 
> That's because although delete_xbc() does:
> 
> 	pr_output = 0;
> 	size = load_xbc_from_initrd(fd, &buf);
> 
> in load_xbc_from_initrd() the error message is printed with printf, not
> printk, so it's not controlled by pr_output:
> 
> 	printf("checksum error: %d != %d\n", csum, rcsum);

Oh, I got it. If there is no bootconfig in initrd, it warns but
that is expected result. 

> 
> Switching that line to printk fixes it, ie. makes the checksum error go
> away, but it seems a bit odd to be using printk in userspace code.

What about pr_err() as perf does? :)
OK, I'll fix the error messages.

Thank you,


> 
> cheers


-- 
Masami Hiramatsu <mhiramat@xxxxxxxxxx>