Skip to content

Comments

format: we need to include the last byte#5977

Closed
SiliconA-Z wants to merge 1 commit intomicrosoft:mainfrom
SiliconA-Z:off-by-one
Closed

format: we need to include the last byte#5977
SiliconA-Z wants to merge 1 commit intomicrosoft:mainfrom
SiliconA-Z:off-by-one

Conversation

@SiliconA-Z
Copy link
Contributor

We skip the last byte, which I guess happens to work for us, but is still improper.

@SiliconA-Z SiliconA-Z requested a review from a team as a code owner December 27, 2025 03:54
@github-project-automation github-project-automation bot moved this to Initial Review in STL Code Reviews Dec 27, 2025
We skip the last byte, which I guess happens to work for us, but is still improper.

Also use int because unsigned char is bad for ARM64 and cannot exceed the value of an unsigned char anyway.
@cpplearner
Copy link
Contributor

You already opened #5942. Why do you create this PR, which fixes the same problem in a different way?

@StephanTLavavej StephanTLavavej added bug Something isn't working format C++20/23 format labels Dec 31, 2025
@StephanTLavavej StephanTLavavej moved this from Initial Review to Work In Progress in STL Code Reviews Dec 31, 2025
@StephanTLavavej
Copy link
Member

Yes, can you please explain what's going on here?

Additionally, we will need test coverage that actually exercises this fix.

PRs are challenging to review when they're dropped on our doorstep with little explanation or context. We want to welcome PRs but we really need more info.

@StephanTLavavej
Copy link
Member

StephanTLavavej commented Jan 30, 2026

This does seem to address a real bug, according to CPINFOEXW structure "Ranges are inclusive", and the structure of the fix looks reasonable at first glance, but we still need to figure out how to exercise this.

@StephanTLavavej
Copy link
Member

Superseded by #6059, thanks @cpplearner.

@github-project-automation github-project-automation bot moved this from Work In Progress to Done in STL Code Reviews Jan 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working format C++20/23 format

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

3 participants