Skip to content

Fix negative bitmap index normalization - #2123

Merged
Vasileios Zois (vazois) merged 3 commits into
mainfrom
vazois/bitmap-negative-index
Sep 10, 2026
Merged

Vasileios Zois (vazois) merged 3 commits into
mainfrom
vazois/bitmap-negative-index

Conversation

@vazois

Copy link
Copy Markdown
Contributor

Summary

  • clamp BITCOUNT and BITPOS negative indices before the start to zero instead of wrapping
  • preserve the BITPOS early bounds check while accepting the valid negative maximum-length boundary
  • add BYTE and BIT regression coverage

Testing

  • BITCOUNT and BITPOS tests (net10.0 Debug)

Copilot AI balanced review requested due to automatic review settings September 10, 2026 00:15

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

BITCOUNT compares raw negative offsets before clamping, producing incorrect results when both indices precede the value.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Updates BITCOUNT/BITPOS negative index handling to clamp before-start offsets to zero and adds regression coverage.

Changes:

  • Revises bitmap offset normalization and BITPOS bounds.
  • Adds BYTE/BIT regression tests.
  • Updates test-side expected-value helpers.

Metadata: Accurate overall; consider adding the [RESP] prefix and linked issue.

File summaries
File Description
BitmapManager.cs Updates normalization and BITPOS bounds.
BitmapManagerBitCount.cs Handles reversed negative BITCOUNT ranges.
GarnetBitmapTests.cs Adds negative-offset regression coverage.
Review details
  • Files reviewed: 3/3 changed files
  • Comments generated: 2
  • Review effort level: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread libs/server/Resp/Bitmap/BitmapManagerBitCount.cs
Comment thread test/standalone/Garnet.test.complexstring/GarnetBitmapTests.cs
@vazois
Vasileios Zois (vazois) merged commit 7084f12 into main Sep 10, 2026
167 of 169 checks passed
@vazois
Vasileios Zois (vazois) deleted the vazois/bitmap-negative-index branch September 10, 2026 18:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants