Merge pull request #46421 from iarspider/iarspider-patches-20241017-3

[LLVM Analyzer] Cleanup analyzer warnings
cms-sw · Oct 28, 2024 · 3e4dead · 3e4dead
2 parents ddb8a3b + 6b495a0
commit 3e4dead
Show file tree

Hide file tree

Showing 6 changed files with 45 additions and 15 deletions.
diff --git a/DataFormats/DTRecHit/interface/DTDriftTimeParameters.icc b/DataFormats/DTRecHit/interface/DTDriftTimeParameters.icc
@@ -4,6 +4,7 @@
 // THIS_CLASS should be #defined accordingly.
 
 #include <string>
+#include <cassert>
 
 #define DT_Cell_Width 42
 #define DT_Cell_HalfWidth 21
@@ -474,6 +475,10 @@ inline void THIS_CLASS::MB_DT_Get_grid_values(
     }
   }
 
+  // To silence LLVM analyzer warning
+  assert(jValue >= 0);
+  assert(iValue >= 0);
+
   if (fabs(Values[iValue] - Var) <= fabs(Var - Values[jValue])) {
     *pi = iValue;
     *pj = jValue;

diff --git a/DataFormats/GeometryCommonDetAlgo/interface/DeepCopyPointerByClone.h b/DataFormats/GeometryCommonDetAlgo/interface/DeepCopyPointerByClone.h
@@ -2,9 +2,10 @@
 #define DeepCopyPointerByClone_H
 
 #include <algorithm>
+#include <cassert>
 
 /** Same as DeepCopyPointer, except that it copies the object
- *  pointed to wsing the clone() virtual copy constructor.
+ *  pointed to using the clone() virtual copy constructor.
  */
 
 template <class T>
@@ -34,17 +35,29 @@ class DeepCopyPointerByClone {
   }
 
   // straight from http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2006/n2027.html
-  DeepCopyPointerByClone(DeepCopyPointerByClone&& other) : theData(other.theData) { other.theData = 0; }
+  DeepCopyPointerByClone(DeepCopyPointerByClone&& other) : theData(other.theData) { other.theData = nullptr; }
   DeepCopyPointerByClone& operator=(DeepCopyPointerByClone&& other) {
     std::swap(theData, other.theData);
     return *this;
   }
 
-  T& operator*() { return *theData; }
-  const T& operator*() const { return *theData; }
+  T& operator*() {
+    assert(theData);
+    return *theData;
+  }
+  const T& operator*() const {
+    assert(theData);
+    return *theData;
+  }
 
-  T* operator->() { return theData; }
-  const T* operator->() const { return theData; }
+  T* operator->() {
+    assert(theData);
+    return theData;
+  }
+  const T* operator->() const {
+    assert(theData);
+    return theData;
+  }
 
   /// to allow test like " if (p) {...}"
   operator bool() const { return theData != 0; }

diff --git a/EventFilter/Utilities/plugins/ExceptionGenerator.cc b/EventFilter/Utilities/plugins/ExceptionGenerator.cc
@@ -178,7 +178,8 @@ namespace evf {
           edm::LogError("TestErrorMessage") << qualifier_;
           break;
         case 8:
-          *pi = 0;  //intentionally caused segfault by assigning null pointer (this produces static-checker warning)
+          [[clang::suppress]] *pi =
+              0;  //intentionally caused segfault by assigning null pointer (this produces static-checker warning)
           break;
         case 9:
           for (unsigned int j = 0; j < intqualifier_ * 1000 * 100; j++) {
@@ -209,8 +210,10 @@ namespace evf {
         case 12: {
           timeval tv_now;
           gettimeofday(&tv_now, nullptr);
-          if ((unsigned)(tv_now.tv_sec - tv_start_.tv_sec) > intqualifier_)
-            *pi = 0;  //intentionally caused segfault by assigning null pointer (this produces static-checker warning)
+          if ((unsigned)(tv_now.tv_sec - tv_start_.tv_sec) > intqualifier_) {
+            [[clang::suppress]] *pi =
+                0;  //intentionally caused segfault by assigning null pointer (this produces static-checker warning)
+          }
         } break;
         case 13: {
           void *vp = malloc(1024);

diff --git a/FWCore/Framework/src/StreamSchedule.cc b/FWCore/Framework/src/StreamSchedule.cc
@@ -1189,9 +1189,10 @@ namespace edm {
         ServiceRegistry::Operate operate(weakToken.lock());
 
         if (iPtr) {
-          //this is used to prioritize this error over one
-          // that happens in EndPath or Accumulate
-          pathErrorPtr->store(new std::exception_ptr(*iPtr));
+          // free previous value of pathErrorPtr, if any;
+          // prioritize this error over one that happens in EndPath or Accumulate
+          auto currentPtr = pathErrorPtr->exchange(new std::exception_ptr(*iPtr));
+          assert(currentPtr == nullptr);
         }
         finishedPaths(*pathErrorPtr, std::move(allPathsHolder), transitionInfo);
       });

diff --git a/FWCore/Services/src/SiteLocalConfigService.cc b/FWCore/Services/src/SiteLocalConfigService.cc
@@ -185,7 +185,11 @@ namespace edm {
 
     std::filesystem::path const SiteLocalConfigService::storageDescriptionPath(
         edm::CatalogAttributes const &aDataCatalog) const {
-      std::string siteconfig_path = std::string(std::getenv("SITECONFIG_PATH"));
+      char *tmp = std::getenv("SITECONFIG_PATH");
+      if (tmp == nullptr) {
+        throw cms::Exception("SiteLocalConfigService") << "SITECONFIG_PATH is not set!";
+      }
+      std::string siteconfig_path = std::string(tmp);
       std::filesystem::path filename_storage;
       //not a cross site use local path given in SITECONFIG_PATH
       if (aDataCatalog.site == aDataCatalog.storageSite) {

diff --git a/HLTrigger/JetMET/plugins/HLTRHemisphere.cc b/HLTrigger/JetMET/plugins/HLTRHemisphere.cc
@@ -177,13 +177,17 @@ void HLTRHemisphere::ComputeHemispheres(std::unique_ptr<std::vector<math::XYZTLo
       if (itemp / j_count == 1) {
         if (count < JETS.size())
           j_temp1 += JETS.at(count);
-        else
+        else {
+          assert(extraJets);  // to silence LLVM analyzer warning
           j_temp1 += extraJets->at(count - JETS.size());
+        }
       } else {
         if (count < JETS.size())
           j_temp2 += JETS.at(count);
-        else
+        else {
+          assert(extraJets);  // to silence LLVM analyzer warning
           j_temp2 += extraJets->at(count - JETS.size());
+        }
       }
       itemp -= j_count * (itemp / j_count);
       j_count /= 2;