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.
