OK florian@

On Mon, Aug 07, 2017 at 11:53:13AM +0200, Mark Kettenis wrote:
> > 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]);
>  
> 

-- 
I'm not entirely sure you are real.

Reply via email to