-
Notifications
You must be signed in to change notification settings - Fork 872
[23624] Allow empty partition list to match against "*" (backport #5989)
#6008
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
Conversation
|
Cherry-pick of 27ce90d has failed: To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally |
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.
Pull Request Overview
This PR fixes partition matching behavior to ensure that empty partition lists (treated as "") correctly match against wildcard partitions ("*"). The implementation refactors all partition matching to use the StringMatching class consistently and ensures uniform empty string handling across platforms (Windows, WinRT, and POSIX).
Key Changes:
- Unified all partition matching logic to use
StringMatchingmethods instead of custom helpers - Added special handling for empty strings matching
"*"wildcards on Windows platform - Removed platform-specific redundant implementations in favor of shared helper functions
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/cpp/utils/StringMatching.cpp | Refactored to use shared do_match_pattern() helper functions and added empty string handling for Windows platform |
| src/cpp/rtps/builtin/discovery/endpoint/EDP.cpp | Replaced custom is_partition_empty() function with calls to StringMatching::matchString() |
| test/unittest/utils/StringMatchingTests.cpp | Added test coverage for empty string matching behavior with wildcards |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
@Mergifyio rebase |
* Refs #23623. Always use StringMatching. Signed-off-by: Miguel Company <miguelcompany@eprosima.com> * Refs #23623. Refactor StringMatching. Signed-off-by: Miguel Company <miguelcompany@eprosima.com> * Refs #23623. Regression unit test.. Signed-off-by: Miguel Company <miguelcompany@eprosima.com> * Refs #23623. Fix StringMatching on Windows. Signed-off-by: Miguel Company <miguelcompany@eprosima.com> * Refs #23624. Avoid unnecessary conditionals. Signed-off-by: Miguel Company <miguelcompany@eprosima.com> --------- Signed-off-by: Miguel Company <miguelcompany@eprosima.com> (cherry picked from commit 27ce90d) # Conflicts: # src/cpp/rtps/builtin/discovery/endpoint/EDP.cpp # src/cpp/utils/StringMatching.cpp
Signed-off-by: Emilio Cuesta <emiliocuesta@eprosima.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Emilio Cuesta Fernandez <emiliocuesta@eprosima.com>
✅ Branch has been successfully rebased |
9aa22c0 to
d0eae7a
Compare
Description
A partition with value
"*"should match with a partition with value"".An empty partition list should be treated as a list with a single partition with value
"".This should also match with a partition with value
"*".This PR refactors the matching of partitions to always use the
StringMatchingclass, and makes the behavior of that class with respect to empty strings consistent between platforms.@Mergifyio backport 3.3.x 3.2.x 2.14.x
Contributor Checklist
Commit messages follow the project guidelines.
The code follows the style guidelines of this project.
Tests that thoroughly check the new feature have been added/Regression tests checking the bug and its fix have been added; the added tests pass locally
N/A: Any new/modified methods have been properly documented using Doxygen.
N/A: Any new configuration API has an equivalent XML API (with the corresponding XSD extension)
Changes are backport compatible: they do NOT break ABI nor change library core behavior.
Changes are API compatible.
N/A: New feature has been added to the
versions.mdfile (if applicable).N/A: New feature has been documented/Current behavior is correctly described in the documentation.
Applicable backports have been included in the description.
Reviewer Checklist
This is an automatic backport of pull request #5989 done by [Mergify](https://mergify.com).