Skip to content

Commit de0cc36

Browse files
committed
GH-49753: [C++][Gandiva]: Incorporated review comments.
- Added extra context error checks in test code. - Removed dead code.
1 parent 7252535 commit de0cc36

3 files changed

Lines changed: 8 additions & 10 deletions

File tree

cpp/src/gandiva/gdv_string_function_stubs.cc

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -485,10 +485,6 @@ const char* gdv_fn_substring_index(int64_t context, const char* txt, int32_t txt
485485
return out;
486486

487487
} else {
488-
if (txt_len < 0) {
489-
*out_len = 0;
490-
return "";
491-
}
492488
memcpy(out, txt, static_cast<size_t>(txt_len));
493489
*out_len = txt_len;
494490
return out;

cpp/src/gandiva/precompiled/string_ops.cc

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1929,7 +1929,7 @@ const char* quote_utf8(gdv_int64 context, const char* in, gdv_int32 in_len,
19291929
// Test multiply overflow for in_len
19301930
if (ARROW_PREDICT_FALSE(
19311931
arrow::internal::MultiplyWithOverflow(2, in_len, &double_len))) {
1932-
gdv_fn_context_set_error_msg(context, "Memory allocation size too large");
1932+
gdv_fn_context_set_error_msg(context, "Memory allocation size too large.");
19331933
*out_len = 0;
19341934
return "";
19351935
}
@@ -2557,9 +2557,6 @@ static inline const char* concat_ws_impl(int64_t context, const char* separator,
25572557
*out_valid = false;
25582558
return "";
25592559
}
2560-
if (state.overflow) {
2561-
return handle_overflow_failure(out_valid, out_len);
2562-
}
25632560
}
25642561

25652562
// Add separator lengths
@@ -2780,7 +2777,8 @@ const char* elt_int32_utf8_utf8_utf8_utf8_utf8(
27802777
FORCE_INLINE
27812778
const char* to_hex_binary(int64_t context, const char* text, int32_t text_len,
27822779
int32_t* out_len) {
2783-
if (ARROW_PREDICT_FALSE(text_len <= 0)) {
2780+
if (ARROW_PREDICT_FALSE(text_len < 0)) {
2781+
gdv_fn_context_set_error_msg(context, "Text length invalid(negative).");
27842782
*out_len = 0;
27852783
return "";
27862784
}
@@ -2789,7 +2787,7 @@ const char* to_hex_binary(int64_t context, const char* text, int32_t text_len,
27892787
// Check multiply overflow for text_len
27902788
if (ARROW_PREDICT_FALSE(
27912789
arrow::internal::MultiplyWithOverflow(2, text_len, &double_len))) {
2792-
gdv_fn_context_set_error_msg(context, "Memory allocation size too large");
2790+
gdv_fn_context_set_error_msg(context, "Memory allocation size too large.");
27932791
*out_len = 0;
27942792
return "";
27952793
}

cpp/src/gandiva/precompiled/string_ops_test.cc

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1168,11 +1168,13 @@ TEST(TestStringOps, TestQuote) {
11681168

11691169
int32_t bad_in_len = std::numeric_limits<int32_t>::max() / 2 + 1;
11701170
out_str = quote_utf8(ctx_ptr, "YYZ", bad_in_len, &out_len);
1171+
EXPECT_EQ(ctx.get_error(), "Memory allocation size too large.");
11711172
EXPECT_EQ(out_len, 0);
11721173
EXPECT_STREQ(out_str, "");
11731174

11741175
bad_in_len = std::numeric_limits<int32_t>::max() / 2 + 20;
11751176
out_str = quote_utf8(ctx_ptr, "ABCDE", bad_in_len, &out_len);
1177+
EXPECT_EQ(ctx.get_error(), "Memory allocation size too large.");
11761178
EXPECT_EQ(out_len, 0);
11771179
EXPECT_STREQ(out_str, "");
11781180
}
@@ -2573,12 +2575,14 @@ TEST(TestStringOps, TestToHex) {
25732575
out_str = to_hex_binary(ctx_ptr, binary_string, bad_text_len, &out_len);
25742576
EXPECT_EQ(out_len, 0);
25752577
EXPECT_STREQ(out_str, "");
2578+
EXPECT_EQ(ctx.get_error(), "Memory allocation size too large.");
25762579
ctx.Reset();
25772580

25782581
int32_t neg_in_len = -20;
25792582
out_str = to_hex_binary(ctx_ptr, binary_string, neg_in_len, &out_len);
25802583
EXPECT_EQ(out_len, 0);
25812584
EXPECT_STREQ(out_str, "");
2585+
EXPECT_EQ(ctx.get_error(), "Text length invalid(negative).");
25822586
ctx.Reset();
25832587
}
25842588

0 commit comments

Comments
 (0)