Skip to content

Conversation

Emik03
Copy link

@Emik03 Emik03 commented May 30, 2025

Fix bounds check in test code

A single bounds check from a class used in test code (DiagonsticPoolBlock.cs) has been corrected.

Description

I was implementing MemoryManager<T> in my own project but needed to find an example implementation of the Pin method to ensure I did everything correctly. Looking through grep.app, I found the file this pull request is about and took note of what it did, but then looking closer I saw a bounds check that looked wrong.

Sure enough, this class does allow you to pin the element that comes after the last. (i.e. block.Pin(memoryOwner.Memory.Length)). I'm aware that this code is only used for testing, so as far as bugs are concerned this is very low priority, but it's better to fix it before it causes problems given that the type is in a shared project, implying that this type may be reused and could cause a future problem.

I hope you don't mind me sending a PR without a corresponding issue, as I believe opening an issue for something this small would only slow both me and the maintainers down. The bounds check that I replaced it with is based on what I've seen the most from the runtime repository to maintain some consistency.

This addresses an erroneous bounds check that allowed the element after the last to be pinned.
@github-actions github-actions bot added the needs-area-label Used by the dotnet-issue-labeler to label those issues which couldn't be triaged automatically label May 30, 2025
@dotnet-policy-service dotnet-policy-service bot added the community-contribution Indicates that the PR has been added by a community member label May 30, 2025
Copy link
Contributor

Thanks for your PR, @@Emik03. Someone from the team will get assigned to your PR shortly and we'll get it reviewed.

@gfoidl gfoidl added area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions and removed needs-area-label Used by the dotnet-issue-labeler to label those issues which couldn't be triaged automatically labels Jun 2, 2025
Copy link
Contributor

Looks like this PR hasn't been active for some time and the codebase could have been changed in the meantime.
To make sure no conflicting changes have occurred, please rerun validation before merging. You can do this by leaving an /azp run comment here (requires commit rights), or by simply closing and reopening.

@dotnet-policy-service dotnet-policy-service bot added the pending-ci-rerun When assigned to a PR indicates that the CI checks should be rerun label Jun 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions community-contribution Indicates that the PR has been added by a community member pending-ci-rerun When assigned to a PR indicates that the CI checks should be rerun

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants