Philip Zeyliger has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12658 )

Change subject: Remove unused THROW_IF_ERROR_WITH_LOGGING, THROW_IF_EXC, and 
RETURN_IF_EXC.
......................................................................


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/12658/1//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/12658/1//COMMIT_MSG@7
PS1, Line 7: Remove unused THROW_IF_ERROR_WITH_LOGGING, THROW_IF_EXC, and 
RETURN_IF_EXC.
> nit: Add "IMPALA-8250: "
Done


http://gerrit.cloudera.org:8080/#/c/12658/1//COMMIT_MSG@9
PS1, Line 9: These macros have no references, so removing them. Found while
> So were these removed purely for cleanup purposes, or was there something w
Added to the commit message. Some of them were broken.

Note how ExceptionClear() is called after CallStaticObjectMethod()...

-    jthrowable exc = (env)->ExceptionOccurred(); \
-    if (exc != nullptr) { \
-      DCHECK((throwable_to_string_id_) != nullptr); \
-      jstring stack = (jstring) 
env->CallStaticObjectMethod(JniUtil::jni_util_class(), \
-          (JniUtil::throwable_to_stack_trace_id()), exc); \
-      jboolean is_copy; \
-      const char* c_stack = \
-          reinterpret_cast<const char*>((env)->GetStringUTFChars(stack, 
&is_copy)); \
-      (env)->ExceptionClear(); \



--
To view, visit http://gerrit.cloudera.org:8080/12658
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic63b498e0e4da57c7ec15cf1ad8070a6afdb3d96
Gerrit-Change-Number: 12658
Gerrit-PatchSet: 1
Gerrit-Owner: Philip Zeyliger <phi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <joemcdonn...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <phi...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <stak...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <t...@apache.org>
Gerrit-Comment-Date: Tue, 05 Mar 2019 19:07:56 +0000
Gerrit-HasComments: Yes

Reply via email to