Daniel Becker has posted comments on this change. ( http://gerrit.cloudera.org:8080/19104 )
Change subject: IMPALA-11640/IMPALA-11641: Workaround errors in shared library build on Ubuntu 18+ ...................................................................... Patch Set 1: (3 comments) http://gerrit.cloudera.org:8080/#/c/19104/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/19104/1//COMMIT_MSG@11 PS1, Line 11: unwind_safeness.cc:76] Check failed: !error failed to find symbol dlopen This line could be surrounded by empty lines to emphasise that it is an example of the error. http://gerrit.cloudera.org:8080/#/c/19104/1/be/src/kudu/util/debug/unwind_safeness.cc File be/src/kudu/util/debug/unwind_safeness.cc: http://gerrit.cloudera.org:8080/#/c/19104/1/be/src/kudu/util/debug/unwind_safeness.cc@36 PS1, Line 36: ((decltype(&func_name))g_orig_ ## func_name)(__VA_ARGS__) To avoid dereferencing a NULL pointer in at least debug builds, we could change this line to DCHECK(g_orig_ ## func_name != nullptr), ((decltype(&func_name))g_orig_ ## func_name)(__VA_ARGS__) The comma operator ensures that the DCHECK is evaluated first but the result of the expression is that of the second operand. Dereferencing a nullptr is technically undefined behaviour so it would be good to avoid it even if the consequence is very probably a simple crash. http://gerrit.cloudera.org:8080/#/c/19104/1/be/src/kudu/util/debug/unwind_safeness.cc@77 PS1, Line 77: CHECK(!error) << "failed to find symbol " << sym << ": " << error; Afaik the error in older systems is reporting the failure of dlsym through dlerror(). Couldn't we more reliably check the result if we also checked whether 'ret' is NULL? -- To view, visit http://gerrit.cloudera.org:8080/19104 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iaab196b3d669ccc12854a98d0dbfbae2b9b91244 Gerrit-Change-Number: 19104 Gerrit-PatchSet: 1 Gerrit-Owner: Joe McDonnell <[email protected]> Gerrit-Reviewer: Daniel Becker <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Michael Smith <[email protected]> Gerrit-Comment-Date: Mon, 10 Oct 2022 12:39:25 +0000 Gerrit-HasComments: Yes
