svn commit: r310138 - head/lib/libc/stdio
John Baldwin
jhb at freebsd.org
Mon Dec 19 17:02:29 UTC 2016
On Friday, December 16, 2016 07:31:28 PM Eric van Gyzen wrote:
> On 12/16/2016 17:44, Warner Losh wrote:
> > On Fri, Dec 16, 2016 at 3:07 PM, John Baldwin <jhb at freebsd.org> wrote:
> >> On Friday, December 16, 2016 04:53:04 PM Eric van Gyzen wrote:
> >>> On 12/16/2016 16:45, John Baldwin wrote:
> >>>> On Friday, December 16, 2016 08:53:26 PM Dimitry Andric wrote:
> >>>>> On 16 Dec 2016, at 20:31, Baptiste Daroussin <bapt at FreeBSD.org> wrote:
> >>>>>>
> >>>>>> On Fri, Dec 16, 2016 at 01:44:51AM +0000, Conrad E. Meyer wrote:
> >>>>>>> Author: cem
> >>>>>>> Date: Fri Dec 16 01:44:50 2016
> >>>>>>> New Revision: 310138
> >>>>>>> URL: https://svnweb.freebsd.org/changeset/base/310138
> >>>>>>>
> >>>>>>> Log:
> >>>>>>> vfprintf(3): Add support for kernel %b format
> >>>>>>>
> >>>>>>> This is a direct port of the kernel %b format.
> >>>>>>>
> >>>>>>> I'm unclear on if (more) non-portable printf extensions will be a
> >>>>>>> problem. I think it's desirable to have userspace formats include all
> >>>>>>> kernel formats, but there may be competing goals I'm not aware of.
> >>>>>>>
> >>>>>>> Reviewed by: no one, unfortunately
> >>>>>>> Sponsored by: Dell EMC Isilon
> >>>>>>> Differential Revision: https://reviews.freebsd.org/D8426
> >>>>>>>
> >>>>>>
> >>>>>> I really don't think it is a good idea, if used in userland it would be make
> >>>>>> more of our code difficult to port elsewhere.
> >>>>>
> >>>>> Indeed, this is a bad idea. These custom format specifiers should be
> >>>>> eliminated, not multiplied. :-)
> >>>>>
> >>>>>
> >>>>>> Other than that, it makes more difficult to use vanilla gcc with out userland.
> >>>>>> and it is adding more complexity to be able to build freebsd from a non freebsd
> >>>>>> system which some people are working on.
> >>>>>>
> >>>>>> Personnaly I would prefer to see those extensions removed from the kernel rather
> >>>>>> than see them available in userland.
> >>>>>
> >>>>> Same here.
> >>>>>
> >>>>>
> >>>>>> Can't we use simple helper function instead?
> >>>>>
> >>>>> Yes, please. Just take the snprintb(3) function from NetBSD:
> >>>>>
> >>>>> http://netbsd.gw.com/cgi-bin/man-cgi?snprintb+3+NetBSD-current
> >>>>
> >>>> In general I agree with something like this instead, but it is quite a bit more
> >>>> tedious to use as you have to run it once to determine the length, allocate a
> >>>> buffer, and then run it again. Calling malloc() for that buffer isn't always
> >>>> convenient in the kernel (though it should be fine in userland). Having it live
> >>>> in printf() itself means the output is generated to the stream without having to
> >>>> manage a variable-sized intermediate buffer.
> >>>
> >>> I imagine most callers can simply use a char[sizeof(fmt)+C] on the stack, where
> >>> C is some constant that I haven't taken the time to calculate, at the risk of
> >>> making myself look foolish and unprofessional.
> >>
> >> Hmm, that might work, but it is still cumbersome. Probably to make things readable
> >> we'd end up with a wrapper:
> >>
> >> printb(uint val, const char *fmt)
> >> {
> >> char buf[strlen(fmt) + C];
> >>
> >> snprintb(...);
> >> printf("%s", buf);
> >> }
> >
> > Sadly this "cure" is worse than the disease.
>
> How about this cure?
>
> printf("reg=%b\n", value, FORMAT);
>
> // versus
>
> char buf[BITMASK_BUFFER_SIZE(FORMAT)];
> printf("reg=%s\n", format_bitmask(buf, sizeof(buf), value, FORMAT));
>
> That doesn't seem so bad.
The trick here is giving FORMAT twice. For code that often uses %b that
is untenable. You would have to make it a macro (or use printb which only
accepts it once which is why I suggested that approach instead). But a
macro moves its definition out of context. Here's an example to think about:
/*
* Here we should probably set up flags indicating
* whether or not various features are available.
* The interesting ones are probably VME, PSE, PAE,
* and PGE. The code already assumes without bothering
* to check that all CPUs >= Pentium have a TSC and
* MSRs.
*/
printf("\n Features=0x%b", cpu_feature,
"\020"
"\001FPU" /* Integral FPU */
"\002VME" /* Extended VM86 mode support */
"\003DE" /* Debugging Extensions (CR4.DE) */
"\004PSE" /* 4MByte page tables */
"\005TSC" /* Timestamp counter */
"\006MSR" /* Machine specific registers */
"\007PAE" /* Physical address extension */
"\010MCE" /* Machine Check support */
"\011CX8" /* CMPEXCH8 instruction */
"\012APIC" /* SMP local APIC */
"\013oldMTRR" /* Previous implementation of MTRR */
"\014SEP" /* Fast System Call */
"\015MTRR" /* Memory Type Range Registers */
"\016PGE" /* PG_G (global bit) support */
"\017MCA" /* Machine Check Architecture */
"\020CMOV" /* CMOV instruction */
"\021PAT" /* Page attributes table */
"\022PSE36" /* 36 bit address space support */
"\023PN" /* Processor Serial number */
"\024CLFLUSH" /* Has the CLFLUSH instruction */
"\025<b20>"
"\026DTS" /* Debug Trace Store */
"\027ACPI" /* ACPI support */
"\030MMX" /* MMX instructions */
"\031FXSR" /* FXSAVE/FXRSTOR */
"\032SSE" /* Streaming SIMD Extensions */
"\033SSE2" /* Streaming SIMD Extensions #2 */
"\034SS" /* Self snoop */
"\035HTT" /* Hyperthreading (see EBX bit 16-23) */
"\036TM" /* Thermal Monitor clock slowdown */
"\037IA64" /* CPU can execute IA64 instructions */
"\040PBE" /* Pending Break Enable */
);
if (cpu_feature2 != 0) {
printf("\n Features2=0x%b", cpu_feature2,
"\020"
"\001SSE3" /* SSE3 */
"\002PCLMULQDQ" /* Carry-Less Mul Quadword */
"\003DTES64" /* 64-bit Debug Trace */
"\004MON" /* MONITOR/MWAIT Instructions */
"\005DS_CPL" /* CPL Qualified Debug Store */
"\006VMX" /* Virtual Machine Extensions */
"\007SMX" /* Safer Mode Extensions */
"\010EST" /* Enhanced SpeedStep */
"\011TM2" /* Thermal Monitor 2 */
"\012SSSE3" /* SSSE3 */
"\013CNXT-ID" /* L1 context ID available */
"\014SDBG" /* IA32 silicon debug */
"\015FMA" /* Fused Multiply Add */
"\016CX16" /* CMPXCHG16B Instruction */
"\017xTPR" /* Send Task Priority Messages*/
"\020PDCM" /* Perf/Debug Capability MSR */
"\021<b16>"
"\022PCID" /* Process-context Identifiers*/
"\023DCA" /* Direct Cache Access */
"\024SSE4.1" /* SSE 4.1 */
"\025SSE4.2" /* SSE 4.2 */
"\026x2APIC" /* xAPIC Extensions */
"\027MOVBE" /* MOVBE Instruction */
"\030POPCNT" /* POPCNT Instruction */
"\031TSCDLT" /* TSC-Deadline Timer */
"\032AESNI" /* AES Crypto */
"\033XSAVE" /* XSAVE/XRSTOR States */
"\034OSXSAVE" /* OS-Enabled State Management*/
"\035AVX" /* Advanced Vector Extensions */
"\036F16C" /* Half-precision conversions */
"\037RDRAND" /* RDRAND Instruction */
"\040HV" /* Hypervisor */
);
}
(and several more of these)
--
John Baldwin
More information about the svn-src-all
mailing list