Add support for binary integer values #236
Conversation
Modifiy the libconfig grammar to include binary integers (and 64-bit integers), prefixed with 0b (or 0B). This also adds a binary integer format so that they can then be output as binary. Binaries are output using a custom formatter, even though some C libraries now support %b/%B format specifiers. This could be improved in the future. The code is based heavily on the hexadecimal handling (so, e.g. only the L serves as an indicator that it should be 64-bits, not the number of zeroes).
lib/util.c
Outdated
| unsigned i = 0; | ||
| while ( (c < 64) && (i < buflen -1) ) | ||
| { | ||
| buf[i] = (val & ( 1ll << (63-c))) ? '1' : '0'; |
There was a problem hiding this comment.
nit: fix spacing
(1ll << (63 - c )))
lib/util.c
Outdated
| i++; | ||
| c++; | ||
| } | ||
| buf[i]=0; |
|
Thanks for the positive reception! Do you know yet what the next version will be (so that the version adding the feature can be documented)? Also, I noticed that the documentation implies that values outside a 32-bit range will automatically be promoted to 64-bit, but as far as I can tell this is not the case for hex. Is this a bug or a conscious decision (maybe because of the MINGW32 workaround potentially causing some degradation in the 32-bit case?). I copied the same behavior as hex for binary, but automatic promotion to 64-bits would probably be better (and also more consistent with documentation), so I'll go ahead and implement that unless you have an objection? |
|
It's probably a bug, so feel free to fix that. All 32- and 64-bit
integer values should behave the same way regardless of display format.
I don't know if that strotull() bug still exists in MinGW...I have not
worked on the Windows port in several years so it may have been fixed by
now.
Next version will be 1.7.4. I can prepare a release after you merge your
changes. We're long overdue for a new release as a few PRs have been merged
since the last one.
…On Mon, Apr 15, 2024 at 10:53 PM Cosmin Deaconu ***@***.***> wrote:
Thanks for the positive reception!
Do you know yet what the next version will be (so that the version adding
the feature can be documented)?
Also, I noticed that the documentation implies that values outside a
32-bit range will automatically be promoted to 64-bit, but as far as I can
tell this is not the case for hex. Is this a bug or a conscious decision
(maybe because of the MINGW32 workaround potentially causing some
degradation in the 32-bit case?). I copied the same behavior as hex for
binary, but automatic promotion to 64-bits would probably be better (and
also more consistent with documentation), so I'll go ahead and implement
that unless you have an objection?
—
Reply to this email directly, view it on GitHub
<#236 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAR4MIYV7L66A22J5BPQW6LY5SVFPAVCNFSM6AAAAABFUSB6PWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDANJYGIZDKMZTGY>
.
You are receiving this because you commented.Message ID:
***@***.***>
--
--
Mark Lindner
http://www.hyperrealm.com/
|
This commit does a few different things:
1) Includes error checking for all integer types (integer, hex, bin)
2) This includes swalling anny errno set by stro[u]ll and restoring
errno to original tate
3) Automatically extends hex and bin to 64 bits even with no L
present. In this case, we only check against INT_MAX since it's
rare you'd want to consider negative numbers here, I think.
We nowalways use the parse_integer/hex/bin functions in the scanner,
which now all fill ok (and eat errno if necessary). They also all take
an L_ok argument, which determines if it's ok have L's at the end of the
digits (this is set to 1 for integer64/hex64/bin64, 0 otherwise).
Note that the previous code was setting errno to 0 on failure rather
than to err_save, which I think was probably a bug, unless I'm missing
something
|
I've gone ahead and tried to make all the integer parsing consistent, including automatic promotion to 64 bits and error detection. This passes tests, but probably needs to be sanity checked a bit more to make sure I didn't introduce any silly bugs. I have also taken a stab at updating the docs. |
| more hexadecimal digits (@samp{0} - @samp{9}, @samp{A} - @samp{F}, | ||
| @samp{a} - @samp{f}). Additionally, octal notation integers (that is, | ||
| those having a leading zero with non-zero value) are also allowed. | ||
| Integers can be represented in one of two ways: as a series of one or more |
There was a problem hiding this comment.
s/one of two/the following/
| library, the trailing `L' is optional; if the integer value exceeds the range | ||
| of a 32-bit integer, it will automatically be interpreted as a 64-bit integer. | ||
| As of version 1.7.4 of the library this behavior is extended to hexadecimal | ||
| (and binary) integers as well. |
There was a problem hiding this comment.
no need for parens around 'and binary'
| @@ -57,6 +57,7 @@ extern "C" { | |||
|
|
|||
| #define CONFIG_FORMAT_DEFAULT 0 | |||
| #define CONFIG_FORMAT_HEX 1 | |||
| #define CONFIG_FORMAT_BIN 2 | |||
There was a problem hiding this comment.
the C++ wrapper needs to be updated as well to update the corresponding enum there
There was a problem hiding this comment.
Please also update the C++ section of the documentation; there's a 'format' enum there as well
This adds support for binary integers to the libconfig grammar, specified by a 0b or 0B prefix. These are treated very similarly to how hex integers are treated (including defining an integer format for them so they may be output, and having an L suffix for 64-bit integers). One might accuse me of grepping for hex and then doing the same thing for binary...
A few salient points: