> Date: Mon, 7 Aug 2017 11:32:55 +0200 (CEST)
> From: Mark Kettenis <mark.kette...@xs4all.nl>
> 
> > Date: Mon, 7 Aug 2017 11:04:51 +0200 (CEST)
> > From: Markus Hennecke <markus-henne...@markus-hennecke.de>
> > 
> > On Thu, 3 Aug 2017, Mark Kettenis wrote:
> > 
> > > Currently clang ignores the "kprintf" format attribute.  I've got a
> > > fix for that, but with that fix it complains about the code fixed in
> > > the diff below.
> > > 
> > > Now our manual page says:
> > > 
> > >    FORMAT OPTIONS
> > >      The kernel functions don't support as many formatting specifiers as 
> > > their
> > >      user space counterparts.  In addition to the floating point 
> > > formatting
> > >      specifiers, the following integer type specifiers are not supported:
> > > 
> > >      %hh    Argument of char type.  This format specifier is accepted by 
> > > the
> > >             kernel but will be handled as %h.
> > > 
> > > So as far as the kernel is concerned, this change is a no-op.
> > > 
> > > ok?
> > > 
> > > P.S. While printing as a short is technically incorrect, it works
> > >      because the arguments are unsigned and promoted to int.
> > > 
> > > 
> > > Index: subr_disk.c
> > > ===================================================================
> > > RCS file: /cvs/src/sys/kern/subr_disk.c,v
> > > retrieving revision 1.230
> > > diff -u -p -r1.230 subr_disk.c
> > > --- subr_disk.c   4 May 2017 22:47:27 -0000       1.230
> > > +++ subr_disk.c   3 Aug 2017 17:38:32 -0000
> > > @@ -1864,7 +1864,7 @@ duid_format(u_char *duid)
> > >   static char duid_str[17];
> > >  
> > >   snprintf(duid_str, sizeof(duid_str),
> > > -     "%02hx%02hx%02hx%02hx%02hx%02hx%02hx%02hx",
> > > +     "%02hhx%02hhx%02hhx%02hhx%02hhx%02hhx%02hhx%02hhx",
> > >       duid[0], duid[1], duid[2], duid[3],
> > >       duid[4], duid[5], duid[6], duid[7]);
> > 
> > This breaks kernel builds with gcc. It looks like gcc in base is too old 
> > for handling %hhx. I tried to build a kernel for armv7 with rev 1.231 
> > today.
> 
> Damn.  Thanks for spotting this.  It is fairly easy to add %hh support
> to our gcc, but maybe we shouldn't.
> 
> So here is an alternative approach.  Just cast to u_short in
> recognition that this is what the kernel actually supports.
> 
> ok?

Or as pirofti@ suggests, simply droppy the 'h' modifier altogether is
safe as well as u_char gets promoted to int in a varargs function.
And clang doesn't want about this one.

Any preference?

> Index: kern/subr_disk.c
> ===================================================================
> RCS file: /cvs/src/sys/kern/subr_disk.c,v
> retrieving revision 1.231
> diff -u -p -r1.231 subr_disk.c
> --- kern/subr_disk.c  6 Aug 2017 19:56:29 -0000       1.231
> +++ kern/subr_disk.c  7 Aug 2017 09:30:29 -0000
> @@ -1864,9 +1864,10 @@ duid_format(u_char *duid)
>       static char duid_str[17];
>  
>       snprintf(duid_str, sizeof(duid_str),
> -         "%02hhx%02hhx%02hhx%02hhx%02hhx%02hhx%02hhx%02hhx",
> -         duid[0], duid[1], duid[2], duid[3],
> -         duid[4], duid[5], duid[6], duid[7]);
> +         "%02hx%02hx%02hx%02hx%02hx%02hx%02hx%02hx",
> +         (u_short)duid[0], (u_short)duid[1], (u_short)duid[2],
> +         (u_short)duid[3], (u_short)duid[4], (u_short)duid[5],
> +         (u_short)duid[6], (u_short)duid[7]);
>  
>       return (duid_str);
>  }

Index: kern/subr_disk.c
===================================================================
RCS file: /cvs/src/sys/kern/subr_disk.c,v
retrieving revision 1.231
diff -u -p -r1.231 subr_disk.c
--- kern/subr_disk.c    6 Aug 2017 19:56:29 -0000       1.231
+++ kern/subr_disk.c    7 Aug 2017 09:50:43 -0000
@@ -1864,7 +1864,7 @@ duid_format(u_char *duid)
        static char duid_str[17];
 
        snprintf(duid_str, sizeof(duid_str),
-           "%02hhx%02hhx%02hhx%02hhx%02hhx%02hhx%02hhx%02hhx",
+           "%02x%02x%02x%02x%02x%02x%02x%02x",
            duid[0], duid[1], duid[2], duid[3],
            duid[4], duid[5], duid[6], duid[7]);
 

Reply via email to