Skip to content
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

[C++] re2::RE2::RE2() result must be checked #33723

Closed
kou opened this issue Jan 17, 2023 · 2 comments · Fixed by #33806
Closed

[C++] re2::RE2::RE2() result must be checked #33723

kou opened this issue Jan 17, 2023 · 2 comments · Fixed by #33806

Comments

@kou
Copy link
Member

kou commented Jan 17, 2023

Describe the enhancement requested

re2::RE2::RE2() may be failed. We should check the re2::RE2::RE2() result with re2::RE2::ok().

For example:

diff --git a/cpp/src/arrow/compute/kernels/scalar_string_ascii.cc b/cpp/src/arrow/compute/kernels/scalar_string_ascii.cc
index d3d0ac3201..b2b9d47c02 100644
--- a/cpp/src/arrow/compute/kernels/scalar_string_ascii.cc
+++ b/cpp/src/arrow/compute/kernels/scalar_string_ascii.cc
@@ -1681,6 +1681,10 @@ struct FindSubstringRegex {
 
   template <typename OutValue, typename... Ignored>
   OutValue Call(KernelContext*, std::string_view val, Status*) const {
+    if (!regex_match_->ok()) {
+      // TODO: Report error
+      return -1;
+    }
     re2::StringPiece piece(val.data(), val.length());
     re2::StringPiece match;
     if (RE2::PartialMatch(piece, *regex_match_, &match)) {

Gandiva also doesn't check re2::RE2::RE2() result.

If re2::RE2::RE2() is failed, a program is crashed like #25633 .

Component(s)

C++, C++ - Gandiva

@js8544
Copy link
Collaborator

js8544 commented Jan 18, 2023

Hi @kou, are you working on this? If not I can take it.

@kou
Copy link
Member Author

kou commented Jan 18, 2023

No. I'm not working on this. Could you work on this?

js8544 added a commit to js8544/arrow that referenced this issue Jan 20, 2023
kou pushed a commit that referenced this issue Jan 22, 2023
### Rationale for this change

RE2 construction needs to be checked.

### What changes are included in this PR?

Check all RE2 object status.

### Are these changes tested?

Tested with existing tests.

### Are there any user-facing changes?

No.
* Closes: #33723

Authored-by: Jin Shang <shangjin1997@gmail.com>
Signed-off-by: Sutou Kouhei <kou@clear-code.com>
@kou kou added this to the 12.0.0 milestone Jan 22, 2023
sjperkins pushed a commit to sjperkins/arrow that referenced this issue Feb 10, 2023
…33806)

### Rationale for this change

RE2 construction needs to be checked.

### What changes are included in this PR?

Check all RE2 object status.

### Are these changes tested?

Tested with existing tests.

### Are there any user-facing changes?

No.
* Closes: apache#33723

Authored-by: Jin Shang <shangjin1997@gmail.com>
Signed-off-by: Sutou Kouhei <kou@clear-code.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment