luis4a0 commented on PR #12086: URL: https://github.com/apache/gluten/pull/12086#issuecomment-4439474400
Closing this — on second look (prompted by reviewer feedback to find a real failing case), the original bounds check is functionally correct, just stylistically odd. `std::string::substr(pos, count)` clamps `count` to `size() - pos`. Since `find()` guarantees `pos + config.size() <= size()` whenever it returns non-npos, the subsequent `substr(pos + config.size(), targetFunction.size())` is always safe and always returns at most the requested count of characters. The equality check against `targetFunction` then either matches the full intended substring or returns false on a clamped (shorter) result — never a wrong answer. Verified empirically with an exhaustive test across 90 (message, target) pairs including various truncations: zero behavioral differences between the two bound-check variants. `std::string_view::substr` would also be safe (only throws when `pos > size()`, which can't happen given the `find()` contract). So there's no bug to fix here, dormant or otherwise. Sorry for the noise; thanks for taking a look. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
