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

util: Add ParseUInt32 and ParseUInt64 #8168

Merged
merged 1 commit into from Jun 9, 2016

Conversation

Projects
None yet
5 participants
@laanwj
Member

laanwj commented Jun 8, 2016

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 some
point.

Also adds tests, and updates developer-notes.md.

util: Add ParseUInt32 and ParseUInt64
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 some
point.

Also adds tests, and updates `developer-notes.md`.
@MarcoFalke

This comment has been minimized.

Show comment
Hide comment
@MarcoFalke

MarcoFalke Jun 8, 2016

Member

utACK e012f3c. I think we also need them in init.cpp

Member

MarcoFalke commented Jun 8, 2016

utACK e012f3c. I think we also need them in init.cpp

@jonasschnelli

This comment has been minimized.

Show comment
Hide comment
@jonasschnelli
Member

jonasschnelli commented Jun 8, 2016

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 &&

This comment has been minimized.

@sipa

sipa Jun 8, 2016

Member

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)

@sipa

sipa Jun 8, 2016

Member

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)

This comment has been minimized.

@sipa

sipa Jun 8, 2016

Member

Ah, ParsePrechecks catches this.

@sipa

sipa Jun 8, 2016

Member

Ah, ParsePrechecks catches this.

This comment has been minimized.

@laanwj

laanwj Jun 8, 2016

Member

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.

@laanwj

laanwj Jun 8, 2016

Member

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.

This comment has been minimized.

@sipa

sipa Jun 8, 2016

Member
@sipa

sipa via email Jun 8, 2016

Member
@sipa

This comment has been minimized.

Show comment
Hide comment
@sipa

sipa Jun 8, 2016

Member

utACK e012f3c

Member

sipa commented Jun 8, 2016

utACK e012f3c

@theuni

This comment has been minimized.

Show comment
Hide comment
@theuni

theuni Jun 8, 2016

Member

@laanwj Thanks for grabbing this so quickly! A generic utility function is the way to go for sure.

utACK e012f3c

Member

theuni commented Jun 8, 2016

@laanwj Thanks for grabbing this so quickly! A generic utility function is the way to go for sure.

utACK e012f3c

@laanwj laanwj merged commit e012f3c into bitcoin:master Jun 9, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

laanwj added a commit that referenced this pull request Jun 9, 2016

Merge #8168: util: Add ParseUInt32 and ParseUInt64
e012f3c util: Add ParseUInt32 and ParseUInt64 (Wladimir J. van der Laan)

codablock added a commit to codablock/dash that referenced this pull request Sep 16, 2017

Merge #8168: util: Add ParseUInt32 and ParseUInt64
e012f3c util: Add ParseUInt32 and ParseUInt64 (Wladimir J. van der Laan)

codablock added a commit to codablock/dash that referenced this pull request Sep 19, 2017

Merge #8168: util: Add ParseUInt32 and ParseUInt64
e012f3c util: Add ParseUInt32 and ParseUInt64 (Wladimir J. van der Laan)

codablock added a commit to codablock/dash that referenced this pull request Dec 22, 2017

Merge #8168: util: Add ParseUInt32 and ParseUInt64
e012f3c util: Add ParseUInt32 and ParseUInt64 (Wladimir J. van der Laan)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment