Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/18134 )
Change subject: IMPALA-9999: Switch to GCC 10.4 ...................................................................... Patch Set 14: (2 comments) http://gerrit.cloudera.org:8080/#/c/18134/14/be/src/udf/udf.cc File be/src/udf/udf.cc: http://gerrit.cloudera.org:8080/#/c/18134/14/be/src/udf/udf.cc@112 PS14, Line 112: user_string_ > Just to learn something new: what was the problem with return ""? Here is the warning (and this has been around a while even on GCC 7.5 on release builds): 01:33:43 In member function ‘const char* impala_udf::FunctionContext::user() const’: 01:33:43 cc1plus: warning: function may return address of local variable [-Wreturn-local-addr] 01:33:43 /data/jenkins/workspace/impala-cdw-master-exhaustive-release/repos/Impala/be/src/udf/udf.cc:286:40: note: declared here 01:33:43 return impl_->state_->connected_user().c_str(); 01:33:43 ^ 01:33:43 In member function ‘const char* impala_udf::FunctionContext::effective_user() const’: 01:33:43 cc1plus: warning: function may return address of local variable [-Wreturn-local-addr] 01:33:43 /data/jenkins/workspace/impala-cdw-master-exhaustive-release/repos/Impala/be/src/udf/udf.cc:286:40: note: declared here This structure is a substitute for RuntimeState when doing UDF development. The code that uses connected_user() is then getting the address of the string's underlying char array and returning it. That is bad if this is a local variable, because the local variable will be destroyed quickly, but the pointer may live on. For the real RuntimeState, connected_user() is pointing at a piece of the query context, and the compiler knows it isn't a local variable. Pointing it at this class member makes it similar to how the normal RuntimeState works and avoids the warning. There are interface changes we could make to eliminate this as well. Since this is only for UDF development, it doesn't impact real workloads. http://gerrit.cloudera.org:8080/#/c/18134/14/be/src/util/parquet-reader.cc File be/src/util/parquet-reader.cc: http://gerrit.cloudera.org:8080/#/c/18134/14/be/src/util/parquet-reader.cc@225 PS14, Line 225: an > nit: a Thanks for catching this. Done -- To view, visit http://gerrit.cloudera.org:8080/18134 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ibe6857b822925226d39fd4d6413457ef6bbaabec Gerrit-Change-Number: 18134 Gerrit-PatchSet: 14 Gerrit-Owner: Joe McDonnell <[email protected]> Gerrit-Reviewer: Abhishek Rawat <[email protected]> Gerrit-Reviewer: Andrew Sherman <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Joe McDonnell <[email protected]> Gerrit-Reviewer: Michael Smith <[email protected]> Gerrit-Comment-Date: Fri, 16 Sep 2022 17:58:26 +0000 Gerrit-HasComments: Yes
