helifu has posted comments on this change. ( http://gerrit.cloudera.org:8080/12294 )
Change subject: Fix compilation warnings under debian8.9 ...................................................................... Patch Set 1: (9 comments) http://gerrit.cloudera.org:8080/#/c/12294/1/src/kudu/cfile/bshuf_block.cc File src/kudu/cfile/bshuf_block.cc: http://gerrit.cloudera.org:8080/#/c/12294/1/src/kudu/cfile/bshuf_block.cc@99 PS1, Line 99: uint32_t mid_key = 0; > In this case, I think size_of_elem_ must be either 1, 2, or 4. Would a defa Done http://gerrit.cloudera.org:8080/#/c/12294/1/src/kudu/cfile/rle_block.h File src/kudu/cfile/rle_block.h: http://gerrit.cloudera.org:8080/#/c/12294/1/src/kudu/cfile/rle_block.h@380 PS1, Line 380: CppType cur_elem = 0; > I'm skeptical of this: sure the previous uninitialized value is harmful, bu 1.compilation warning: /mnt/ddb/2/helif/apache/kudu/src/kudu/cfile/rle_block.h:389:7: warning: ‘cur_elem’ may be used uninitialized in this function [-Wmaybe-uninitialized] if (cur_elem > target) { ^ 2.hmm i can't agree with the statement that initializing objects wastes CPU unless they are very very complex, and selectively enable or disable certain types of diagnostics either. It's important to be clear and controlled, so i think giving a default value is the simplest way. http://gerrit.cloudera.org:8080/#/c/12294/1/src/kudu/common/row_operations.cc File src/kudu/common/row_operations.cc: PS1: > The changes here shouldn't be necessary; it's not possible for ReadIssetBit i changed to gcc8.2.1, and the warning info is still there: ======================================================== In file included from /mnt/ddb/2/helif/apache/kudu/src/kudu/common/row.h:32:0, from /mnt/ddb/2/helif/apache/kudu/src/kudu/common/row_operations.cc:28: /mnt/ddb/2/helif/apache/kudu/src/kudu/util/bitmap.h: In member function ‘kudu::Status kudu::RowOperationsPBDecoder::DecodeUpdateOrDelete(const kudu::ClientServerMapping&, kudu::DecodedRowOperation*)’: /mnt/ddb/2/helif/apache/kudu/src/kudu/util/bitmap.h:57:25: warning: ‘client_null_map’ may be used uninitialized in this function [-Wmaybe-uninitialized] return bitmap[idx >> 3] & (1 << (idx & 7)); ^ /mnt/ddb/2/helif/apache/kudu/src/kudu/common/row_operations.cc:406:18: note: ‘client_null_map’ was declared here const uint8_t* client_null_map; http://gerrit.cloudera.org:8080/#/c/12294/1/src/kudu/consensus/leader_election.cc File src/kudu/consensus/leader_election.cc: http://gerrit.cloudera.org:8080/#/c/12294/1/src/kudu/consensus/leader_election.cc@304 PS1, Line 304: CHECK_OK(vote_counter_->GetDecision(&decision)); > Likewise, decision will always be set if this returns success. /mnt/ddb/2/helif/apache/kudu/src/kudu/consensus/leader_election.cc: In member function ‘void kudu::consensus::LeaderElection::CheckForDecision()’: /mnt/ddb/2/helif/apache/kudu/src/kudu/consensus/leader_election.cc:168:20: warning: ‘decision’ may be used uninitialized in this function [-Wmaybe-uninitialized] message(message) { ^ /mnt/ddb/2/helif/apache/kudu/src/kudu/consensus/leader_election.cc:303:20: note: ‘decision’ was declared here ElectionVote decision; ^ http://gerrit.cloudera.org:8080/#/c/12294/1/src/kudu/consensus/log-test.cc File src/kudu/consensus/log-test.cc: http://gerrit.cloudera.org:8080/#/c/12294/1/src/kudu/consensus/log-test.cc@331 PS1, Line 331: int offset = 0; > Could we use a default case with LOG(FATAL) instead of this? Done http://gerrit.cloudera.org:8080/#/c/12294/1/src/kudu/fs/log_block_manager-test-util.cc File src/kudu/fs/log_block_manager-test-util.cc: PS1: > Same feedback. In file included from /mnt/ddb/2/helif/apache/kudu/src/kudu/util/env.h:26:0, from /mnt/ddb/2/helif/apache/kudu/src/kudu/fs/log_block_manager-test-util.h:25, from /mnt/ddb/2/helif/apache/kudu/src/kudu/fs/log_block_manager-test-util.cc:18: /mnt/ddb/2/helif/apache/kudu/src/kudu/util/status.h: In member function ‘kudu::Status kudu::fs::LBMCorruptor::PreallocateFullContainer()’: /mnt/ddb/2/helif/apache/kudu/src/kudu/util/status.h:36:34: warning: ‘c’ may be used uninitialized in this function [-Wmaybe-uninitialized] const ::kudu::Status& _s = (s); \ ^ /mnt/ddb/2/helif/apache/kudu/src/kudu/fs/log_block_manager-test-util.cc:111:20: note: ‘c’ was declared here const Container* c; ^ In file included from /mnt/ddb/2/helif/apache/kudu/src/kudu/util/env.h:26:0, from /mnt/ddb/2/helif/apache/kudu/src/kudu/fs/log_block_manager-test-util.h:25, from /mnt/ddb/2/helif/apache/kudu/src/kudu/fs/log_block_manager-test-util.cc:18: /mnt/ddb/2/helif/apache/kudu/src/kudu/util/status.h: In member function ‘kudu::Status kudu::fs::LBMCorruptor::AddUnpunchedBlockToFullContainer()’: /mnt/ddb/2/helif/apache/kudu/src/kudu/util/status.h:36:34: warning: ‘c’ may be used uninitialized in this function [-Wmaybe-uninitialized] const ::kudu::Status& _s = (s); \ ^ /mnt/ddb/2/helif/apache/kudu/src/kudu/fs/log_block_manager-test-util.cc:145:20: note: ‘c’ was declared here const Container* c; ^ http://gerrit.cloudera.org:8080/#/c/12294/1/src/kudu/hms/hms_client-test.cc File src/kudu/hms/hms_client-test.cc: PS1: > What was the issue here? /mnt/ddb/2/helif/apache/kudu/src/kudu/hms/hms_client-test.cc: In member function ‘virtual void kudu::hms::HmsClientTest_TestLargeObjects_Test::TestBody()’: /mnt/ddb/2/helif/apache/kudu/src/kudu/hms/hms_client-test.cc:301:36: warning: ‘*((void*)& protection +4)’ may be used uninitialized in this function [-Wmaybe-uninitialized] *protection); ^ /mnt/ddb/2/helif/apache/kudu/src/kudu/hms/hms_client-test.cc: In member function ‘virtual void kudu::hms::HmsClientTest_TestHmsOperations_Test::TestBody()’: /mnt/ddb/2/helif/apache/kudu/src/kudu/hms/hms_client-test.cc:124:36: warning: ‘*((void*)& protection +4)’ may be used uninitialized in this function [-Wmaybe-uninitialized] *protection); ^ http://gerrit.cloudera.org:8080/#/c/12294/1/src/kudu/integration-tests/log_verifier.cc File src/kudu/integration-tests/log_verifier.cc: http://gerrit.cloudera.org:8080/#/c/12294/1/src/kudu/integration-tests/log_verifier.cc@151 PS1, Line 151: int64_t expected_term = kNotOnThisServer; > What was the warning here exactly? I thought the default constructor for bo 1.the original warning is: /mnt/ddb/2/helif/apache/kudu/src/kudu/integration-tests/log_verifier.cc: In member function ‘kudu::Status kudu::LogVerifier::VerifyCommittedOpIdsMatch()’: /mnt/ddb/2/helif/apache/kudu/src/kudu/integration-tests/log_verifier.cc:158:16: warning: ‘*((void*)& expected_term +8)’ may be used uninitialized in this function [-Wmaybe-uninitialized] } else if (this_ts_term != expected_term) { ^ 2.after setting the variable to boots::none, the warning is still there: /mnt/ddb/2/helif/apache/kudu/src/kudu/integration-tests/log_verifier.cc: In member function ‘kudu::Status kudu::LogVerifier::VerifyCommittedOpIdsMatch()’: /mnt/ddb/2/helif/apache/kudu/src/kudu/integration-tests/log_verifier.cc:158:16: warning: ‘*((void*)& expected_term +8)’ may be used uninitialized in this function [-Wmaybe-uninitialized] } else if (this_ts_term != expected_term) { ^ 3.it seems that it is a gcc issue:https://stackoverflow.com/questions/21755206/how-to-get-around-gcc-%E2%80%98void-b-4%E2%80%99-may-be-used-uninitialized-in-this-funct so i trashed the boost::optional away. http://gerrit.cloudera.org:8080/#/c/12294/1/src/kudu/rpc/server_negotiation.cc File src/kudu/rpc/server_negotiation.cc: http://gerrit.cloudera.org:8080/#/c/12294/1/src/kudu/rpc/server_negotiation.cc@703 PS1, Line 703: security::VerificationResult res = security::VerificationResult::VALID; > Could you use a default case statement? Done -- To view, visit http://gerrit.cloudera.org:8080/12294 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2464ff7938f79d090ebd1676da2108ce7dae4ca5 Gerrit-Change-Number: 12294 Gerrit-PatchSet: 1 Gerrit-Owner: helifu <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: helifu <[email protected]> Gerrit-Comment-Date: Fri, 01 Feb 2019 06:45:29 +0000 Gerrit-HasComments: Yes
