From 49994f7bc3f7a2088aab40ff4fd74102adeb3d42 Mon Sep 17 00:00:00 2001 From: Andrew Hutchings Date: Fri, 6 Dec 2019 14:59:17 +0000 Subject: [PATCH] Fix warnings found in DEBUG combined build Fixes: * Irrelevant where conditions * Irrelevant const * A potential infinite loop in treenode * Bad implicit case fallthroughs * Explicit markings for required case fallthroughs * Unused variables * Unused function Also disabled some warnings for now which we should fix later. --- CMakeLists.txt | 2 ++ dbcon/ddlpackage/ddlpkg.cpp | 1 + dbcon/dmlpackage/row.cpp | 2 +- dbcon/execplan/calpontselectexecutionplan.h | 24 ++++++++++----------- dbcon/execplan/operator.h | 2 +- dbcon/execplan/returnedcolumn.h | 4 ++-- dbcon/execplan/treenode.h | 2 +- dbcon/joblist/distributedenginecomm.cpp | 1 + oamapps/mcsadmin/mcsadmin.cpp | 3 --- primitives/blockcache/stats.cpp | 4 ++-- tools/dbloadxml/inputmgr.cpp | 2 +- utils/common/hasher.h | 16 ++++++++++++++ utils/compress/idbcompress.cpp | 11 ---------- utils/dataconvert/dataconvert.h | 4 ++-- utils/joiner/tuplejoiner.cpp | 3 +++ utils/threadpool/prioritythreadpool.cpp | 2 +- utils/windowfunction/wf_udaf.cpp | 4 +++- utils/windowfunction/windowfunctiontype.h | 6 ++++++ versioning/BRM/extentmap.cpp | 6 +++--- writeengine/shared/we_brm.cpp | 4 ++++ 20 files changed, 62 insertions(+), 41 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 531e7842b..c493e489f 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -197,6 +197,8 @@ ELSE () # Remove visibility flag for now as it breaks Ubuntu 18.05 and we need to # fix our libraries anyway STRING(REPLACE "-fvisibility=hidden" "" CMAKE_CXX_FLAGS ${CMAKE_CXX_FLAGS}) + # TODO: Re-enable these and fix the warnings they generate + MY_CHECK_AND_SET_COMPILER_FLAG("-Wno-ignored-qualifiers -Wno-overloaded-virtual -Wno-vla -Wno-non-virtual-dtor -Wno-extra" DEBUG) ENDIF() SET (ENGINE_LDFLAGS "-Wl,--no-as-needed -Wl,--add-needed") SET (ENGINE_COMMON_LIBS messageqcpp loggingcpp configcpp idbboot ${Boost_LIBRARIES} xml2 pthread rt libmysql_client) diff --git a/dbcon/ddlpackage/ddlpkg.cpp b/dbcon/ddlpackage/ddlpkg.cpp index 03794a304..24404c52b 100644 --- a/dbcon/ddlpackage/ddlpkg.cpp +++ b/dbcon/ddlpackage/ddlpkg.cpp @@ -130,6 +130,7 @@ ColumnType::ColumnType(int type) : case DDL_BIGINT: fPrecision = 19; + break; case DDL_UNSIGNED_BIGINT: fPrecision = 20; diff --git a/dbcon/dmlpackage/row.cpp b/dbcon/dmlpackage/row.cpp index 5d8e95101..45a02c37d 100644 --- a/dbcon/dmlpackage/row.cpp +++ b/dbcon/dmlpackage/row.cpp @@ -96,7 +96,7 @@ const DMLColumn* Row::get_ColumnAt( unsigned int index ) const { const DMLColumn* columnPtr = 0; - if ( index >= 0 && index < fColumnList.size() ) + if ( index < fColumnList.size() ) { columnPtr = fColumnList[index]; } diff --git a/dbcon/execplan/calpontselectexecutionplan.h b/dbcon/execplan/calpontselectexecutionplan.h index cf31b0711..0e6689349 100644 --- a/dbcon/execplan/calpontselectexecutionplan.h +++ b/dbcon/execplan/calpontselectexecutionplan.h @@ -209,7 +209,7 @@ public: /** * Are we in local PM only query mode? */ - const uint32_t localQuery() const + uint32_t localQuery() const { return fLocalQuery; } @@ -301,7 +301,7 @@ public: /** * location of this select */ - const int location () const + int location () const { return fLocation; } @@ -313,7 +313,7 @@ public: /** * dependence of this select */ - const bool dependent() const + bool dependent() const { return fDependent; } @@ -382,7 +382,7 @@ public: /** session id * */ - const uint32_t sessionID() const + uint32_t sessionID() const { return fSessionID; } @@ -394,7 +394,7 @@ public: /** transaction id * */ - const int txnID() const + int txnID() const { return fTxnID; } @@ -457,7 +457,7 @@ public: fTraceFlags = traceFlags; } - const uint32_t statementID() const + uint32_t statementID() const { return fStatementID; } @@ -490,7 +490,7 @@ public: fDerivedTableList = derivedTableList; } - const bool distinct() const + bool distinct() const { return fDistinct; } @@ -503,7 +503,7 @@ public: { fOverrideLargeSideEstimate = over; } - const bool overrideLargeSideEstimate() const + bool overrideLargeSideEstimate() const { return fOverrideLargeSideEstimate; } @@ -534,7 +534,7 @@ public: { fSubType = subType; } - const uint64_t subType() const + uint64_t subType() const { return fSubType; } @@ -556,7 +556,7 @@ public: { fLimitStart = limitStart; } - const uint64_t limitStart() const + uint64_t limitStart() const { return fLimitStart; } @@ -565,7 +565,7 @@ public: { fLimitNum = limitNum; } - const uint64_t limitNum() const + uint64_t limitNum() const { return fLimitNum; } @@ -592,7 +592,7 @@ public: { fOrderByThreads = number; } - const uint32_t orderByThreads() const + uint32_t orderByThreads() const { return fOrderByThreads; } diff --git a/dbcon/execplan/operator.h b/dbcon/execplan/operator.h index a8119fd6e..d6f5af0ba 100644 --- a/dbcon/execplan/operator.h +++ b/dbcon/execplan/operator.h @@ -148,7 +148,7 @@ protected: * F&E framework * ***********************************************************/ public: - virtual const OpType op() const + virtual OpType op() const { return fOp; } diff --git a/dbcon/execplan/returnedcolumn.h b/dbcon/execplan/returnedcolumn.h index e0819b03c..9f9e1f03c 100644 --- a/dbcon/execplan/returnedcolumn.h +++ b/dbcon/execplan/returnedcolumn.h @@ -354,7 +354,7 @@ private: * F&E framework * ******************************************************************/ public: - const uint32_t inputIndex() const + uint32_t inputIndex() const { return fInputIndex; } @@ -362,7 +362,7 @@ public: { fInputIndex = inputIndex; } - const uint32_t outputIndex() const + uint32_t outputIndex() const { return fOutputIndex; } diff --git a/dbcon/execplan/treenode.h b/dbcon/execplan/treenode.h index f2e1c1076..2eb431aa2 100644 --- a/dbcon/execplan/treenode.h +++ b/dbcon/execplan/treenode.h @@ -187,7 +187,7 @@ typedef SP_IDB_Regex SP_CNX_Regex; inline std::string removeTrailing0(char* val, uint32_t length) { char* ptr = val; - uint32_t i = 0; + int64_t i = 0; bool decimal_point = false; for (; i < length; i++, ptr++) diff --git a/dbcon/joblist/distributedenginecomm.cpp b/dbcon/joblist/distributedenginecomm.cpp index d061058f3..056dbfe9a 100644 --- a/dbcon/joblist/distributedenginecomm.cpp +++ b/dbcon/joblist/distributedenginecomm.cpp @@ -833,6 +833,7 @@ void DistributedEngineComm::write(uint32_t senderID, ByteStream& msg) case BATCH_PRIMITIVE_CREATE: /* Disable flow control initially */ msg << (uint32_t) - 1; + /* FALLTHRU */ case BATCH_PRIMITIVE_DESTROY: case BATCH_PRIMITIVE_ADD_JOINER: diff --git a/oamapps/mcsadmin/mcsadmin.cpp b/oamapps/mcsadmin/mcsadmin.cpp index cdaaa9b8a..0b9e500cc 100644 --- a/oamapps/mcsadmin/mcsadmin.cpp +++ b/oamapps/mcsadmin/mcsadmin.cpp @@ -7973,9 +7973,6 @@ int processCommand(string* arguments) devicenetworklist.push_back(devicenetworkconfig); } - DeviceNetworkList::iterator pt = devicenetworklist.begin(); - DeviceNetworkList::iterator endpt = devicenetworklist.end(); - if ( devicenetworklist.empty() ) { cout << endl << "No modules to stop." << endl << endl; diff --git a/primitives/blockcache/stats.cpp b/primitives/blockcache/stats.cpp index 3e74f9ad4..29ab88ccd 100644 --- a/primitives/blockcache/stats.cpp +++ b/primitives/blockcache/stats.cpp @@ -257,7 +257,7 @@ Stats::~Stats() void Stats::touchedLBID(uint64_t lbid, pthread_t thdid, uint32_t session) { - if (lbid < 0 || session == 0) return; + if (session == 0) return; mutex::scoped_lock lk(traceFileMapMutex); TraceFileMap_t::iterator iter = traceFileMap.find(session); @@ -274,7 +274,7 @@ void Stats::touchedLBID(uint64_t lbid, pthread_t thdid, uint32_t session) void Stats::markEvent(const uint64_t lbid, const pthread_t thdid, const uint32_t session, const char event) { - if (lbid < 0 || session == 0) return; + if (session == 0) return; mutex::scoped_lock lk(traceFileMapMutex); TraceFileMap_t::iterator iter = traceFileMap.find(session); diff --git a/tools/dbloadxml/inputmgr.cpp b/tools/dbloadxml/inputmgr.cpp index da23c1242..24736fc82 100644 --- a/tools/dbloadxml/inputmgr.cpp +++ b/tools/dbloadxml/inputmgr.cpp @@ -142,7 +142,7 @@ bool InputMgr::input(int argc, char** argv) return false; } } - + /* FALLTHRU */ case 'd': case 's': case 'f': diff --git a/utils/common/hasher.h b/utils/common/hasher.h index d93694767..e70f62600 100644 --- a/utils/common/hasher.h +++ b/utils/common/hasher.h @@ -178,9 +178,11 @@ public: { case 3: k1 ^= tail[2] << 16; + /* FALLTHRU */ case 2: k1 ^= tail[1] << 8; + /* FALLTHRU */ case 1: k1 ^= tail[0]; @@ -255,21 +257,27 @@ public: { case 15: k2 ^= uint64_t(tail[14]) << 48; + /* FALLTHRU */ case 14: k2 ^= uint64_t(tail[13]) << 40; + /* FALLTHRU */ case 13: k2 ^= uint64_t(tail[12]) << 32; + /* FALLTHRU */ case 12: k2 ^= uint64_t(tail[11]) << 24; + /* FALLTHRU */ case 11: k2 ^= uint64_t(tail[10]) << 16; + /* FALLTHRU */ case 10: k2 ^= uint64_t(tail[9]) << 8; + /* FALLTHRU */ case 9: k2 ^= uint64_t(tail[8]) << 0; @@ -277,27 +285,35 @@ public: k2 = rotl64(k2, 33); k2 *= c1; h2 ^= k2; + /* FALLTHRU */ case 8: k1 ^= uint64_t(tail[7]) << 56; + /* FALLTHRU */ case 7: k1 ^= uint64_t(tail[6]) << 48; + /* FALLTHRU */ case 6: k1 ^= uint64_t(tail[5]) << 40; + /* FALLTHRU */ case 5: k1 ^= uint64_t(tail[4]) << 32; + /* FALLTHRU */ case 4: k1 ^= uint64_t(tail[3]) << 24; + /* FALLTHRU */ case 3: k1 ^= uint64_t(tail[2]) << 16; + /* FALLTHRU */ case 2: k1 ^= uint64_t(tail[1]) << 8; + /* FALLTHRU */ case 1: k1 ^= uint64_t(tail[0]) << 0; diff --git a/utils/compress/idbcompress.cpp b/utils/compress/idbcompress.cpp index 7eff8fe96..c5be0f09e 100644 --- a/utils/compress/idbcompress.cpp +++ b/utils/compress/idbcompress.cpp @@ -88,17 +88,6 @@ void initCompressedDBFileHeader(void* hdrBuf, int compressionType, int hdrSize) hdr->fHeader.fHeaderSize = hdrSize; } -void log(const string& s) -{ - logging::MessageLog logger((logging::LoggingID())); - logging::Message message; - logging::Message::Args args; - - args.add(s); - message.format(args); - logger.logErrorMessage(message); -} - } // namespace diff --git a/utils/dataconvert/dataconvert.h b/utils/dataconvert/dataconvert.h index efa4f5353..b3654bc9b 100644 --- a/utils/dataconvert/dataconvert.h +++ b/utils/dataconvert/dataconvert.h @@ -338,9 +338,9 @@ bool isTimestampValid ( uint64_t second, uint64_t microsecond ) // 0x7FFFFFFF. So enforce the same restriction here. // TODO: We however store the seconds portion of the timestamp in // 44 bits, so change this limit when the server supports higher values. - if ( second >= MIN_TIMESTAMP_VALUE && second <= MAX_TIMESTAMP_VALUE ) + if ( second <= MAX_TIMESTAMP_VALUE ) { - if ( microsecond >= 0 && microsecond <= 999999 ) + if ( microsecond <= 999999 ) { valid = true; } diff --git a/utils/joiner/tuplejoiner.cpp b/utils/joiner/tuplejoiner.cpp index 34d3702e6..dc35f8ca9 100644 --- a/utils/joiner/tuplejoiner.cpp +++ b/utils/joiner/tuplejoiner.cpp @@ -749,6 +749,7 @@ void TupleJoiner::doneInserting() case CalpontSystemCatalog::UFLOAT: { uniquer.insert(*(int64_t*)&dval); + break; } default: { @@ -1104,6 +1105,7 @@ void TupleJoiner::updateCPData(const Row& r) case CalpontSystemCatalog::UFLOAT: { uval = *(uint64_t*)&dval; + break; } default: { @@ -1136,6 +1138,7 @@ void TupleJoiner::updateCPData(const Row& r) case CalpontSystemCatalog::UFLOAT: { val = *(int64_t*)&dval; + break; } default: { diff --git a/utils/threadpool/prioritythreadpool.cpp b/utils/threadpool/prioritythreadpool.cpp index 37ce34ffc..f752ded3b 100644 --- a/utils/threadpool/prioritythreadpool.cpp +++ b/utils/threadpool/prioritythreadpool.cpp @@ -277,7 +277,7 @@ void PriorityThreadPool::threadFcn(const Priority preferredQueue) throw() void PriorityThreadPool::sendErrorMsg(uint32_t id, uint32_t step, primitiveprocessor::SP_UM_IOSOCK sock) { ISMPacketHeader ism; - PrimitiveHeader ph = {0}; + PrimitiveHeader ph = {}; ism.Status = logging::primitiveServerErr; ph.UniqueID = id; diff --git a/utils/windowfunction/wf_udaf.cpp b/utils/windowfunction/wf_udaf.cpp index 0eb753152..af45d7c5f 100644 --- a/utils/windowfunction/wf_udaf.cpp +++ b/utils/windowfunction/wf_udaf.cpp @@ -67,7 +67,9 @@ boost::shared_ptr WF_udaf::makeFunction(int id, const string return func; } -WF_udaf::WF_udaf(WF_udaf& rhs) : fUDAFContext(rhs.getContext()), +WF_udaf::WF_udaf(WF_udaf& rhs) : + WindowFunctionType(rhs.functionId(), rhs.functionName()), + fUDAFContext(rhs.getContext()), bInterrupted(rhs.getInterrupted()), fDistinct(rhs.getDistinct()) { diff --git a/utils/windowfunction/windowfunctiontype.h b/utils/windowfunction/windowfunctiontype.h index ecf8f694e..9260cc4a2 100644 --- a/utils/windowfunction/windowfunctiontype.h +++ b/utils/windowfunction/windowfunctiontype.h @@ -149,6 +149,12 @@ public: { return fFunctionId; } + + const std::string functionName() const + { + return fFunctionName; + } + void functionId(int id) { fFunctionId = id; diff --git a/versioning/BRM/extentmap.cpp b/versioning/BRM/extentmap.cpp index f153ff667..ea43a1053 100644 --- a/versioning/BRM/extentmap.cpp +++ b/versioning/BRM/extentmap.cpp @@ -1839,7 +1839,7 @@ int ExtentMap::lookupLocal(int OID, uint32_t partitionNum, uint16_t segmentNum, #endif int entries, i, offset; - if (OID < 0 || fileBlockOffset < 0) + if (OID < 0) { log("ExtentMap::lookup(): OID and FBO must be >= 0", logging::LOG_TYPE_DEBUG); throw invalid_argument("ExtentMap::lookup(): OID and FBO must be >= 0"); @@ -1892,7 +1892,7 @@ int ExtentMap::lookupLocal_DBroot(int OID, uint16_t dbroot, uint32_t partitionNu #endif int entries, i, offset; - if (OID < 0 || fileBlockOffset < 0) + if (OID < 0) { log("ExtentMap::lookup(): OID and FBO must be >= 0", logging::LOG_TYPE_DEBUG); throw invalid_argument("ExtentMap::lookup(): OID and FBO must be >= 0"); @@ -1955,7 +1955,7 @@ int ExtentMap::lookupLocalStartLbid(int OID, #endif int entries, i; - if (OID < 0 || fileBlockOffset < 0) + if (OID < 0) { log("ExtentMap::lookupLocalStartLbid(): OID and FBO must be >= 0", logging::LOG_TYPE_DEBUG); diff --git a/writeengine/shared/we_brm.cpp b/writeengine/shared/we_brm.cpp index 61b9cff90..7c90b86f4 100644 --- a/writeengine/shared/we_brm.cpp +++ b/writeengine/shared/we_brm.cpp @@ -1749,15 +1749,19 @@ int BRMWrapper::writeVB(IDBDataFile* pSourceFile, const VER_t transID, const OID { case ERR_DEADLOCK: rc = ERR_BRM_DEAD_LOCK; + break; case ERR_VBBM_OVERFLOW: rc = ERR_BRM_VB_OVERFLOW; + break; case ERR_NETWORK: rc = ERR_BRM_NETWORK; + break; case ERR_READONLY: rc = ERR_BRM_READONLY; + break; default: rc = ERR_BRM_WR_VB_ENTRY;