Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Use different defaults for different architectures to avoid integer o… #8

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Use different defaults for different architectures to avoid integer o… #8

wants to merge 1 commit into from

Conversation

andrewstucki
Copy link
Contributor

Fixes #4 by specifying two different aggregation counts, 1 for 32 bit platforms, one for 64 bit.

@a8m
Copy link
Owner

a8m commented Oct 20, 2018 •
edited
Loading

Hey @andrewstucki, thanks for your contribution.
I'm not sure I want to add these extra files just for one variable. What do you think about adding something like this:

var maxAggregationCount = math.MaxInt32
if strconv.IntSize == 64 {
  maxAggregationCount = math.MaxUint32
}

Update: my other suggestion is to just set it to math.MaxInt32 and add an explanation in the comment above (see the current comment).

@a8m
Copy link
Owner

a8m commented Nov 1, 2018

Any update on this @andrewstucki?

@magiconair
Copy link

Any progress here. this is still broken on 32 bit platforms.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

constant 4294967295 overflows int when building for 32-bit
3 participants
@andrewstucki @a8m @magiconair