Page MenuHomeFreeBSD

libc: Implement N2630.
ClosedPublic

Authored by des on Aug 19 2023, 11:46 PM.
Tags
None
Referenced Files
Unknown Object (File)
Mon, Jul 22, 11:25 AM
Unknown Object (File)
Tue, Jul 9, 6:09 PM
Unknown Object (File)
Tue, Jul 9, 5:10 PM
Unknown Object (File)
Mon, Jul 8, 4:41 AM
Unknown Object (File)
Mon, Jul 8, 4:20 AM
Unknown Object (File)
Mon, Jul 8, 4:05 AM
Unknown Object (File)
Sat, Jul 6, 6:50 AM
Unknown Object (File)
Fri, Jul 5, 6:36 PM

Details

Summary

This adds formatted input/output of binary integer numbers to the printf(), scanf(), and strtol() families, including their wide-character counterparts.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped
Build Status
Buildable 53312
Build 50203: arc lint + arc unit

Event Timeline

des requested review of this revision.Aug 19 2023, 11:46 PM

With all the code repetitions, you'd think a macro might not be bad to have for all the places that need to do this parsing.
It's no worse than what we have today, but as the number of cases have grown, it's starting to be difficult to manage.

lib/libc/iconv/_strtol.h
96

'0' || == '1'

also would work, but I'm not sure it would generate better code and so might not be worth breaking the pattern with the other bases.
And should it be an 'else if'?

This revision is now accepted and ready to land.Aug 21 2023, 5:14 PM

Also, it might make sense to include a pointer to what N2630 is or a spec or something. Google doesn't give me good search results when I look for it without getting kinda specific. A simple URL for the commit message would do wonders.

Also, it might make sense to include a pointer to what N2630 is or a spec or something

Yeah, and reference it in the commit message subject too. N2630 doesn't mean much to me but something like "base 2 support for printf/scanf et al" would be useful

I'm not convinced using macros for these functions is going to be straightforward / make things more clear.

lib/libc/stdio/vfscanf.c
439

should p == start on the next line for style(9)?

Reimplement parseint() as a finite state machine.

This revision now requires review to proceed.Aug 21 2023, 9:23 PM
This revision is now accepted and ready to land.Aug 21 2023, 9:49 PM
This revision now requires review to proceed.Aug 26 2023, 4:40 PM

Fix count in "0b" / "0x" case.

des marked an inline comment as done.Aug 26 2023, 5:12 PM

Fix remaining bugs revealed by improved tests.

Still thing that having a 'is_prefix_binary()' macro would make the code cleaner and avoid a lot of duplication (that admittedly we put up with for hex, there should be one for that too), but I lost that argument in the reviews...

This revision is now accepted and ready to land.Aug 28 2023, 3:24 PM
In D41511#948412, @imp wrote:

Still thing that having a 'is_prefix_binary()' macro would make the code cleaner and avoid a lot of duplication (that admittedly we put up with for hex, there should be one for that too), but I lost that argument in the reviews...

Most of the added complexity arises from the fact that the binary prefix is also a valid hexadecimal number... in fact, any binary number, with or without prefix, is also a valid unprefixed hexadecimal number. And we need to handle all cases with at most one character of look-ahead since that's all we're guaranteed to be able to push back to the stream.

des marked an inline comment as done.Aug 28 2023, 3:36 PM
This revision was automatically updated to reflect the committed changes.