Support 32-bit archs like ARM7 #125
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
2hdddg
suggested changes
Jul 31, 2020
neo4j/internal/packstream/packer.go
Outdated
| @@ -127,7 +127,7 @@ func (p *Packer) writeListHeader(l int, shortOffset, longOffset byte) error { | |||
| hdr = hdr[:1+2] | |||
| hdr[0] = longOffset + 1 | |||
| binary.BigEndian.PutUint16(hdr[1:], uint16(l)) | |||
| case l < math.MaxUint32: | |||
| case uint32(l) < uint32(math.MaxUint32): | |||
There was a problem hiding this comment.
uint32(l) will truncate l to fit into uint32 and the comparison will always succeed
l needs to be int64 instead of int to compile and work properly on 32/64 bits systems.
There was a problem hiding this comment.
Thanks for the feedback! Didn't know that int64 would work on a 32-bit system
neo4j/internal/packstream/packer.go
Outdated
| @@ -164,7 +164,7 @@ func (p *Packer) writeBytes(b []byte) error { | |||
| hdr = hdr[:1+2] | |||
| hdr[0] = 0xcd | |||
| binary.BigEndian.PutUint16(hdr[1:], uint16(l)) | |||
| case l < 0x100000000: | |||
| case uint32(l) <= uint32(0xffffffff): | |||
* cast to int64 since values will overflow an int32 * int64 will work on a 32-bit system: https://groups.google.com/forum/#!topic/golang-nuts/mtnn-01Dh_I
This would be great to prevent regressions!
I'd rather not sign the CLA and don't need the attribution but getting this issue fixed would be nice. This code change is trivial so you're welcome to take it and make it your own. |
|
Fixed in another PR |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Before this change, builds of v1.8.0 failed:
$ GOARCH=arm GOARM=7 go build ./... # github.com/neo4j/neo4j-go-driver/neo4j/internal/packstream neo4j/internal/packstream/packer.go:130:10: constant 4294967295 overflows int neo4j/internal/packstream/packer.go:167:9: constant 4294967296 overflows intThis PR fixes that