-
Notifications
You must be signed in to change notification settings - Fork 10k
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
Fix Prefix Matching Logic in PrefixContainer #58068
base: main
Are you sure you want to change the base?
Conversation
…if the prefix matches exactly with keys
Updated the BinarySearch method in PrefixContainer to ensure that an exact match of a prefix does not incorrectly qualify as a valid prefix. Enhanced comments to clarify the flow and decision-making process when checking for delimiters. This change improves the accuracy of prefix matching for hierarchical key structures.
…x if the Prefix is equal to the keys
@dotnet-policy-service agree |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A small JIT-trick for better machine code.
This commit enhances the performance of the BinarySearch method by eliminating unnecessary boundary checks. The condition if ((uint)candidate.Length > (uint)prefix.Length) ensures that the index access candidate[prefix.Length] is safe, and the JIT compiler can now safely elide the boundary check, improving execution efficiency. Co-authored-by: Günther Foidl <gue@korporal.at>
I didn't know that, but it makes sense, thanks! |
The nice thing on contributing: you help to improve the product and get the opportunity to learn something new. A win - win situation 😃. Thanks for the PR 👍🏻 |
Yessss, C# and .NET have been my focus of study since the beginning of my interest in technology and until now I don't even know half of its power. This is my first contribution and I already loved it. Thanks! |
Love it, welcome @sami-daniel! |
/azp run |
Azure Pipelines successfully started running 3 pipeline(s). |
Ubuntu failure is unrelated; it failed during node installation. cc: @dotnet/aspnet-build
|
/azp run |
Commenter does not have sufficient privileges for PR 58068 in repo dotnet/aspnetcore |
/azp run |
Azure Pipelines successfully started running 3 pipeline(s). |
Fix Prefix Matching Logic in PrefixContainer
Description
This PR modifies the
BinarySearch
method inPrefixContainer
to ensure that an exact match of a prefix does not incorrectly qualify as a valid prefix.Now the ContainsPrefix method no longer gives a false positive if it has a prefix with the same name as the Action method parameter.
Fixes #57637