From owner-svn-src-all@freebsd.org Sat Dec 17 01:31:33 2016 Return-Path: Delivered-To: svn-src-all@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 75F69C8384E; Sat, 17 Dec 2016 01:31:33 +0000 (UTC) (envelope-from vangyzen@FreeBSD.org) Received: from smtp.vangyzen.net (hotblack.vangyzen.net [IPv6:2607:fc50:1000:7400:216:3eff:fe72:314f]) by mx1.freebsd.org (Postfix) with ESMTP id 47067956; Sat, 17 Dec 2016 01:31:33 +0000 (UTC) (envelope-from vangyzen@FreeBSD.org) Received: from ford.home.vangyzen.net (unknown [76.164.15.242]) by smtp.vangyzen.net (Postfix) with ESMTPSA id 63C565647F; Fri, 16 Dec 2016 19:31:32 -0600 (CST) Subject: Re: svn commit: r310138 - head/lib/libc/stdio To: Warner Losh , John Baldwin References: <201612160144.uBG1ipjW016736@repo.freebsd.org> <13059937.h5mayX8aKo@ralph.baldwin.cx> <9e255301-f663-a96c-68c7-e6d1a3d1db8c@FreeBSD.org> <3160837.brVkGGj5yS@ralph.baldwin.cx> Cc: Dimitry Andric , Baptiste Daroussin , "Conrad E. Meyer" , src-committers , "svn-src-all@freebsd.org" , "svn-src-head@freebsd.org" From: Eric van Gyzen Message-ID: Date: Fri, 16 Dec 2016 19:31:28 -0600 User-Agent: Mozilla/5.0 (X11; FreeBSD amd64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 17 Dec 2016 01:31:33 -0000 On 12/16/2016 17:44, Warner Losh wrote: > On Fri, Dec 16, 2016 at 3:07 PM, John Baldwin 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 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. Eric