-
Notifications
You must be signed in to change notification settings - Fork 38.2k
util: Add ParseUInt32 and ParseUInt64 #8168
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Add error and range-checking parsers for unsigned 32 and 64 bit numbers. The 32-bit variant is required for parsing sequence numbers from the command line in `bitcoin-tx` (see bitcoin#8164 for discussion). I've thrown in the 64-bit variant as a bonus, as I'm sure it will be needed at some point. Also adds tests, and updates `developer-notes.md`.
|
utACK e012f3c. I think we also need them in init.cpp |
|
utACK e012f3c |
| // Note that strtoul returns a *unsigned long int*, so even if it doesn't report a over/underflow | ||
| // we still have to check that the returned value is within the range of an *uint32_t*. On 64-bit | ||
| // platforms the size of these types may be different. | ||
| return endp && *endp == 0 && !errno && |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of checking whether *endp == 0, shouldn't you check whether endp == str.c_str() + str.size() (otherwise you'd accept strings that have a 0 inside)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, ParsePrechecks catches this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, that solution would be somewhat more elegant. Indeed, ParsePrechecks checks some number-parsing preconditions such as that one.
I'm thinking about (at some point) implementing these functions from scratch instead of calling out to the C API. It wouldn't even be much longer, as there are just too many exceptions. This still doesn't exclude strto*l accepting locale-specific separators, for example. Not a big deal for current uses, but I hate such surprises.
In any case introducing them here (with tests) allows swapping out the implementation later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
|
utACK e012f3c |
e012f3c util: Add ParseUInt32 and ParseUInt64 (Wladimir J. van der Laan)
e012f3c util: Add ParseUInt32 and ParseUInt64 (Wladimir J. van der Laan)
e012f3c util: Add ParseUInt32 and ParseUInt64 (Wladimir J. van der Laan)
Add error and range-checking parsers for unsigned 32 and 64 bit numbers.
The 32-bit variant is required for parsing sequence numbers from the command line in
bitcoin-tx(see #8164 for discussion). I've thrown in the 64-bit variant as a bonus, as I'm sure it will be needed at somepoint.
Also adds tests, and updates
developer-notes.md.