Repository navigation
r.stats: Use a 64-bit counter for cell counts - #7839
Closed
Pranav-error wants to merge 2 commits into
Closed
Pranav-error wants to merge 2 commits into
Pranav-error wants to merge 2 commits into
Conversation
The per-category and total counters were long, which is 32 bits on Windows, so a map with more than about 2.1 billion cells wrapped and r.stats printed negative counts. The reporter's maps were 105063x74530, or roughly 7.8 billion cells. Use grass_int64 for the counters and the comparison variables, and print with PRId64 the way r.info already does. Fixes OSGeo#7769
github-actions
Bot
added
raster
Contributor
|
There is a PR already open for this issue: #7790 |
Contributor
Author
|
Ah, missed that — thanks for pointing it out. #7790 predates this and is already green, so I'll close in favor of it. Sorry for the duplicate work. |
This file contains hidden or 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
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.
Fixes #7769.
The per-category and total counters in
r.statswerelong. That is 64 bits on Linux and macOS but 32 bits on Windows, so a map with more than about 2.1 billion cells wraps and the counts print negative. The reporter's maps are 105063x74530, about 7.8 billion cells, which matches the negative values in the issue.This is @marisn's diagnosis from the issue thread; the change is to act on it.
struct Node.count,total_countand the two comparison variables becomegrass_int64, and the count is printed withPRId64the wayr.infoalready printstotal_cells. The JSON path already went through a double and is unchanged.Verification
I cannot reproduce the overflow on macOS, because
longis already 64 bits there — that is the reason the bug is Windows-only, and it is also why I have not added a test: triggering it needs a map of more than 2^31 cells.What I did check:
clang-formatreports no changesr.statsoutput is byte for byte identical tomainon a small map for-c,-cwith two inputs,-cp, andformat=json, so the counter and format changes do not alter normal outputraw_stats.calso casts tolong, but those are CELL category values rather than counts, so they are left alone.I used an AI assistant while preparing this. I understand the change and can explain it.