Re: svn commit: r310138 - head/lib/libc/stdio

[David Chisnall]

On 22 Dec 2016, at 23:02, Baptiste Daroussin  wrote:
> 
> I think it is pretty clear that there are too many people requesting the 
> revert
> for the revert not to be done.

Even if this feature is desired, the implementation in the patch is broken and 
should be reverted until a correct implementation (one that doesn’t break the 
first time user code calls register_printf_*) is done.

David



smime.p7s
Description: S/MIME cryptographic signature

Re: svn commit: r310138 - head/lib/libc/stdio

[Baptiste Daroussin]

On Wed, Dec 21, 2016 at 11:02:12PM +0100, Jilles Tjoelker wrote:
> On Tue, Dec 20, 2016 at 06:04:31PM -0800, Conrad Meyer wrote:
> > On Tue, Dec 20, 2016 at 5:56 PM, Adrian Chadd  
> > wrote:
> > > Here's my reason for removal.
> 
> > > Plenty of us are looking to be able to build bits of the BSD source
> > > tree as part of other non FreeBSD systems, especially if they're
> > > involved in bootstrapping.
> 
> > Understood, however:
> 
> > > That means that it needs to be compilable
> > > by a non-FreeBSD-modified compiler. Ideally this means we'd stick to
> > > mostly POSIX options source code that we can compile with unmodified
> > > compilers, and we push non-standard stuff into otherly-named
> > > functions.
> 
> > Yeah, this isn't actually a problem.  printf("%b", foo) compiles fine
> > with non-modified compilers.
> 
> It compiles only if you disable format string warnings that should not
> be disabled for any serious software development, in my humble opinion.
> It will build, but not in a way I can call "fine".
> 
> This indeed makes it very hard to justify extensions to format strings.
> Special formatting will need to use new functions.
> 

I think it is pretty clear that there are too many people requesting the revert
for the revert not to be done.

Bapt


signature.asc
Description: PGP signature

Re: svn commit: r310138 - head/lib/libc/stdio

[Jilles Tjoelker]

On Tue, Dec 20, 2016 at 06:04:31PM -0800, Conrad Meyer wrote:
> On Tue, Dec 20, 2016 at 5:56 PM, Adrian Chadd  wrote:
> > Here's my reason for removal.

> > Plenty of us are looking to be able to build bits of the BSD source
> > tree as part of other non FreeBSD systems, especially if they're
> > involved in bootstrapping.

> Understood, however:

> > That means that it needs to be compilable
> > by a non-FreeBSD-modified compiler. Ideally this means we'd stick to
> > mostly POSIX options source code that we can compile with unmodified
> > compilers, and we push non-standard stuff into otherly-named
> > functions.

> Yeah, this isn't actually a problem.  printf("%b", foo) compiles fine
> with non-modified compilers.

It compiles only if you disable format string warnings that should not
be disabled for any serious software development, in my humble opinion.
It will build, but not in a way I can call "fine".

This indeed makes it very hard to justify extensions to format strings.
Special formatting will need to use new functions.

-- 
Jilles Tjoelker
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r310138 - head/lib/libc/stdio

[Pedro Giffuni]

On 20/12/2016 18:50, Conrad Meyer wrote:

I didn't get the same conclusion from the thread — I haven't seen a
persuasive argument for removal.

Best,
Conrad


It's not standard or even a GNU extension  (which we sometimes carry for 
compatibility). We don't want to encourage it's use either so it doesn't 
really belong in libc.


+1 for removal.

Pedro.
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r310138 - head/lib/libc/stdio

[Juli Mallett]

On Tue, Dec 20, 2016 at 6:04 PM, Conrad Meyer  wrote:
> Hi Adrian,
>
> On Tue, Dec 20, 2016 at 5:56 PM, Adrian Chadd  wrote:
>> Here's my reason for removal.
>>
>> Plenty of us are looking to be able to build bits of the BSD source
>> tree as part of other non FreeBSD systems, especially if they're
>> involved in bootstrapping.
>
> Understood, however:
>
>> That means that it needs to be compilable
>> by a non-FreeBSD-modified compiler. Ideally this means we'd stick to
>> mostly POSIX options source code that we can compile with unmodified
>> compilers, and we push non-standard stuff into otherly-named
>> functions.
>
> Yeah, this isn't actually a problem.  printf("%b", foo) compiles fine
> with non-modified compilers.

I want FreeBSD tools to avoid non-standard extensions to standard APIs
where possible.  Non-standard APIs are fine, because they're easy to
provide one's own implementations along with portable software, but
building in quirky behaviour to core POSIX/C/whatever interfaces
invites pain.  In this case, the cost-benefit ratio is all out of
whack.  Now, %b is unusual enough that I don't have to worry that,
say, a bootstrap tool will use it, but it's also unusual enough that
the benefit here is elusive.

I would love to see this reverted and snprintb instead.
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r310138 - head/lib/libc/stdio

[Conrad Meyer]

Hi Adrian,

On Tue, Dec 20, 2016 at 5:56 PM, Adrian Chadd  wrote:
> Here's my reason for removal.
>
> Plenty of us are looking to be able to build bits of the BSD source
> tree as part of other non FreeBSD systems, especially if they're
> involved in bootstrapping.

Understood, however:

> That means that it needs to be compilable
> by a non-FreeBSD-modified compiler. Ideally this means we'd stick to
> mostly POSIX options source code that we can compile with unmodified
> compilers, and we push non-standard stuff into otherly-named
> functions.

Yeah, this isn't actually a problem.  printf("%b", foo) compiles fine
with non-modified compilers.

Best,
Conrad
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r310138 - head/lib/libc/stdio

[Adrian Chadd]

Hi,

Here's my reason for removal.

Plenty of us are looking to be able to build bits of the BSD source
tree as part of other non FreeBSD systems, especially if they're
involved in bootstrapping. That means that it needs to be compilable
by a non-FreeBSD-modified compiler. Ideally this means we'd stick to
mostly POSIX options source code that we can compile with unmodified
compilers, and we push non-standard stuff into otherly-named
functions.

If things are harder then we end up with even more reasons people
resist migrating to FreeBSD and instead just continue to use Linux as
their platform of choice. It already cross-builds and does most of
what people want without getting in their way.

This change is one of those which encourage making that effort harder,
and I'd like to make sure we don't do that. Building bits of the
FreeBSD tree on non-FreeBSD systems (even
non-same-version-FreeBSD-system trees) is increasingly painful.

THanks,



-adrian
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r310138 - head/lib/libc/stdio

[Ian Lepore]

On Tue, 2016-12-20 at 15:50 -0800, Conrad Meyer wrote:
> I didn't get the same conclusion from the thread — I haven't seen a
> persuasive argument for removal.
> 
> Best,
> Conrad
> 

You're kidding, right?  Can you cite even a single message that
supports the change?  My memory is that everyone who voiced an opinion
said it was a bad idea, then the thread devolved into discussions of
alternatives and how they were bad ideas too.  That latter bit doesn't
change the basic fact that everyone started by agreeing this was a bad
idea for userland.

-- Ian

> On Mon, Dec 19, 2016 at 2:23 PM, Adrian Chadd  > wrote:
> > 
> > [snip]
> > 
> > tl;dr - can we revert it from stdio for now so we don't end up
> > having
> > people use this?
> > 
> > 
> > 
> > -adrian
> 
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r310138 - head/lib/libc/stdio

[Conrad Meyer]

I didn't get the same conclusion from the thread — I haven't seen a
persuasive argument for removal.

Best,
Conrad

On Mon, Dec 19, 2016 at 2:23 PM, Adrian Chadd  wrote:
> [snip]
>
> tl;dr - can we revert it from stdio for now so we don't end up having
> people use this?
>
>
>
> -adrian
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r310138 - head/lib/libc/stdio

[John Baldwin]

On Monday, December 19, 2016 02:23:08 PM Adrian Chadd wrote:
> [snip]
> 
> tl;dr - can we revert it from stdio for now so we don't end up having
> people use this?

I agree with that.  I think in userland something like snprintb() is ok.
In the kernel I'd rather keep '%b'.

-- 
John Baldwin
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r310138 - head/lib/libc/stdio

[Adrian Chadd]

[snip]

tl;dr - can we revert it from stdio for now so we don't end up having
people use this?



-adrian
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r310138 - head/lib/libc/stdio

[John Baldwin]

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  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 +, 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 */
 

Re: svn commit: r310138 - head/lib/libc/stdio

[Bruce Evans]

On Sat, 17 Dec 2016, Dimitry Andric wrote:


On 17 Dec 2016, at 12:46, David Chisnall  wrote:


On 16 Dec 2016, at 19:31, Baptiste Daroussin  wrote:


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.


Why?  You???ll get some spurious warnings about printf, but that???s all.


Unfortunately, we compile large parts of the tree with -Werror.  Thus,
"spurious warnings" will break the build, leaving the user two options:
disabling -Wformat warnings, or disabling -Werror altogether, neither of
which are very recommendable.

As far as I know, there is no -Wno-error-on-undefined-printf-specifiers.
It would also be hard to implement, since after any undefined specifiers
have been encountered, you cannot reason about the following ones
anymore either.

IMHO, if people want to use non-standard specifiers, let them define
their own almost_printf_but_not_quite() functions, and forgo any format
checking.


That would be worse than breaking format checking for the selected set
of printf()s.  It gives even more unportability.  %b is a BSDism that
would be detected at compile time on systems without support for %b in
printf().  almost_printf_but_not_quite() is a FreeBSDism that would
be detected at compile time on systems without the function.  Using
it breaks portability even to other BSD systems including previous
versions of FreeBSD.

People who want to use non-standard specifiers added support to check
them the compiler.

Bruce___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r310138 - head/lib/libc/stdio

[Dimitry Andric]

On 17 Dec 2016, at 12:46, David Chisnall  wrote:
> 
> On 16 Dec 2016, at 19:31, Baptiste Daroussin  wrote:
>> 
>> 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.
> 
> Why?  You’ll get some spurious warnings about printf, but that’s all.

Unfortunately, we compile large parts of the tree with -Werror.  Thus,
"spurious warnings" will break the build, leaving the user two options:
disabling -Wformat warnings, or disabling -Werror altogether, neither of
which are very recommendable.

As far as I know, there is no -Wno-error-on-undefined-printf-specifiers.
It would also be hard to implement, since after any undefined specifiers
have been encountered, you cannot reason about the following ones
anymore either.

IMHO, if people want to use non-standard specifiers, let them define
their own almost_printf_but_not_quite() functions, and forgo any format
checking.

-Dimitry



signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: svn commit: r310138 - head/lib/libc/stdio

[David Chisnall]

On 16 Dec 2016, at 19:31, Baptiste Daroussin  wrote:
> 
> 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.

Why?  You’ll get some spurious warnings about printf, but that’s all.  Our 
printf (like the glibc one) already supports user-defined extensions via 
register_printf_function (for which, I note, we don’t have a man page), so 
third-party code also has some of these warnings if they’ve registered other 
printf handlers.

I’d actually consider that to be the biggest argument against adding %b 
support: we support users adding their own interpretation of %b via 
register_printf_function and this will break anyone third-party code where 
people do this. This commit is doubly bad, because not only does it change our 
ABI, it doesn’t document the fact.

The code in this commit is also simply broken.  It does not add a corresponding 
handler in xprintf.c, so as soon as someone calls register_printf_function with 
*any* argument, printf’s ability to handle %b will be broken in a 
difficult-to-debug way.

David



smime.p7s
Description: S/MIME cryptographic signature

Re: svn commit: r310138 - head/lib/libc/stdio

[Adrian Chadd]

... just have printf_freebsd in libutil; have it know about our
extended fmt types.

then we just have to port libutil to a target platform.


-a


On 16 December 2016 at 17:31, Eric van Gyzen  wrote:
> 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 +, 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
>
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r310138 - head/lib/libc/stdio

[Eric van Gyzen]

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 +, 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
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r310138 - head/lib/libc/stdio

[Warner Losh]

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 +, 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.

Warner
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r310138 - head/lib/libc/stdio

[John Baldwin]

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 +, 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);
}

-- 
John Baldwin
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r310138 - head/lib/libc/stdio

[Joerg Sonnenberger]

On Fri, Dec 16, 2016 at 02:45:19PM -0800, John Baldwin wrote:
> 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.

Why do you need to determine the length? It's not like people write
novells in the kernel to describe bit fields. A reasonable sized stack
buffer covers pretty much all the interesting cases.

Joerg
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r310138 - head/lib/libc/stdio

[Eric van Gyzen]

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 +, 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.

Eric
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r310138 - head/lib/libc/stdio

[John Baldwin]

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 +, 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.

-- 
John Baldwin
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r310138 - head/lib/libc/stdio

[Eric van Gyzen]

On 12/16/2016 14:07, Ian Lepore wrote:
> On Fri, 2016-12-16 at 20:31 +0100, Baptiste Daroussin wrote:
>> On Fri, Dec 16, 2016 at 01:44:51AM +, 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.
>>
>> 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.
>>
>> Can't we use simple helper function instead?
>>
>> Best regards,
>> Bapt
> 
> I'll add a big +1 for the concept of eliminating the extensions from
> the kernel instead of extending them to userland.  People ask why
> freebsd can't be built using standard tools on foreign build hosts, and
> these non-standard extensions are part of the reason why.

I strongly agree.  Thanks for mentioning snprintb, Dimitry; I hadn't seen that 
one.

Eric
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r310138 - head/lib/libc/stdio

[Ian Lepore]

On Fri, 2016-12-16 at 20:31 +0100, Baptiste Daroussin wrote:
> On Fri, Dec 16, 2016 at 01:44:51AM +, 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.
> 
> 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.
> 
> Can't we use simple helper function instead?
> 
> Best regards,
> Bapt

I'll add a big +1 for the concept of eliminating the extensions from
the kernel instead of extending them to userland.  People ask why
freebsd can't be built using standard tools on foreign build hosts, and
these non-standard extensions are part of the reason why.

-- Ian

___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r310138 - head/lib/libc/stdio

[Dimitry Andric]

On 16 Dec 2016, at 20:31, Baptiste Daroussin  wrote:
> 
> On Fri, Dec 16, 2016 at 01:44:51AM +, 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

-Dimitry



signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: svn commit: r310138 - head/lib/libc/stdio

[Baptiste Daroussin]

On Fri, Dec 16, 2016 at 01:44:51AM +, 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.

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.

Can't we use simple helper function instead?

Best regards,
Bapt


signature.asc
Description: PGP signature

svn commit: r310138 - head/lib/libc/stdio

[Conrad E. Meyer]

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

Modified:
  head/lib/libc/stdio/vfprintf.c

Modified: head/lib/libc/stdio/vfprintf.c
==
--- head/lib/libc/stdio/vfprintf.c  Fri Dec 16 01:42:51 2016
(r310137)
+++ head/lib/libc/stdio/vfprintf.c  Fri Dec 16 01:44:50 2016
(r310138)
@@ -611,6 +611,37 @@ reswitch:  switch (ch) {
case 'z':
flags |= SIZET;
goto rflag;
+   case 'b':
+   {
+   const char *q;
+   int anybitset, bit;
+
+   ulval = (u_int)GETARG(int);
+   cp = GETARG(char *);
+
+   q = __ultoa(ulval, buf + BUF, *cp++, 0, xdigs_lower);
+   PRINT(q, buf + BUF - q);
+
+   if (ulval == 0)
+   break;
+
+   for (anybitset = 0; *cp;) {
+   bit = *cp++;
+   if (ulval & (1 << (bit - 1))) {
+   PRINT(anybitset ? "," : "<", 1);
+   q = cp;
+   for (; (bit = *cp) > ' '; ++cp)
+   continue;
+   PRINT(q, cp - q);
+   anybitset = 1;
+   } else
+   for (; *cp > ' '; ++cp)
+   continue;
+   }
+   if (anybitset)
+   PRINT(">", 1);
+   }
+   continue;
case 'C':
flags |= LONGINT;
/*FALLTHROUGH*/
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"