Skip to content

Fix bounds check in test code #62180

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.

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

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

Fix bounds check in test code #62180

wants to merge 1 commit into from

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
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants