dmitry-chirkov-dremio commented on code in PR #49471:
URL: https://github.com/apache/arrow/pull/49471#discussion_r2909018113
##########
cpp/src/gandiva/gdv_string_function_stubs.cc:
##########
@@ -407,14 +407,17 @@ const char* gdv_fn_substring_index(int64_t context, const
char* txt, int32_t txt
}
}
- if (static_cast<int32_t>(abs(cnt)) <= static_cast<int32_t>(occ.size()) &&
cnt > 0) {
+ // Use int64_t to avoid undefined behavior with abs(INT_MIN)
+ int64_t abs_cnt = (cnt < 0) ? -static_cast<int64_t>(cnt) :
static_cast<int64_t>(cnt);
Review Comment:
I'd leave as is:
1. The int64_t fix is more robust - it handles ALL negative values
correctly, not just INT_MIN
2. It's the same number of lines - early exit adds 5 lines, current fix adds
3 lines
3. The current fix also simplifies the existing code - removes redundant
static_cast<int32_t> casts
The early-exit approach only guards against the specific crash values, while
the int64_t approach fixes the underlying type-safety issue.
--
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]
