Merge pull request #28756 from pieterdavid/stripunpacker_exceptions

Remove exceptions from the strip unpacker
cms-sw · Jan 29, 2020 · 6114880 · 6114880
2 parents cf33acd + f0c4e61
commit 6114880
Show file tree

Hide file tree

Showing 27 changed files with 1,242 additions and 1,547 deletions.
diff --git a/Alignment/LaserAlignment/plugins/LaserAlignmentEventFilter.cc b/Alignment/LaserAlignment/plugins/LaserAlignmentEventFilter.cc
@@ -67,7 +67,15 @@ bool LaserAlignmentEventFilter::filter(edm::StreamID sid, edm::Event& iEvent, co
       continue;
 
     // construct FEDBuffer
-    sistrip::FEDBuffer buffer(input.data(), input.size());
+    const auto st_buffer = sistrip::preconstructCheckFEDBuffer(input);
+    if (sistrip::FEDBufferStatusCode::SUCCESS != st_buffer) {
+      throw cms::Exception("FEDBuffer") << st_buffer << " (check debug output for more details)";
+    }
+    sistrip::FEDBuffer buffer{input};
+    const auto st_chan = buffer.findChannels();
+    if (sistrip::FEDBufferStatusCode::SUCCESS != st_chan) {
+      throw cms::Exception("FEDBuffer") << st_chan << " (check debug output for more details)";
+    }
     if (not buffer.doChecks(true)) {
       edm::LogWarning("LaserAlignmentEventFilter") << "FED Buffer check fails for FED ID " << *ifed << ".";
       continue;

diff --git a/CalibTracker/SiStripAPVAnalysis/src/ApvAnalysisFactory.cc b/CalibTracker/SiStripAPVAnalysis/src/ApvAnalysisFactory.cc
@@ -153,7 +153,7 @@ void ApvAnalysisFactory::updatePair(uint32_t detId, size_t pairNumber, const edm
 
         for (size_t istrip = startStrip; istrip < stopStrip; istrip++) {
           if (in.data.size() <= istrip)
-            tmpRawDigi.data.push_back(0);
+            tmpRawDigi.data.push_back(SiStripRawDigi(0));
           else
             tmpRawDigi.data.push_back(in.data[istrip]);  //maybe dangerous
         }
@@ -183,7 +183,7 @@ void ApvAnalysisFactory::update(uint32_t detId, const edm::DetSet<SiStripRawDigi
 
       for (size_t istrip = startStrip; istrip < stopStrip; istrip++) {
         if (in.data.size() <= istrip)
-          tmpRawDigi.data.push_back(0);
+          tmpRawDigi.data.push_back(SiStripRawDigi(0));
         else
           tmpRawDigi.data.push_back(in.data[istrip]);  //maybe dangerous
       }

diff --git a/DQM/SiStripMonitorHardware/interface/FEDErrors.hh b/DQM/SiStripMonitorHardware/interface/FEDErrors.hh
@@ -148,7 +148,7 @@ public:
   bool fillFatalFEDErrors(const FEDRawData& aFedData, const unsigned int aPrintDebug);
 
   //expensive check: fatal but kept separate
-  bool fillCorruptBuffer(const sistrip::FEDBuffer* aBuffer);
+  bool fillCorruptBuffer(const sistrip::FEDBuffer& aBuffer);
 
   //FE/Channel check: rate of channels with error (only considering connected channels)
   float fillNonFatalFEDErrors(const sistrip::FEDBuffer* aBuffer, const SiStripFedCabling* aCabling = nullptr);
@@ -165,11 +165,11 @@ public:
                      const bool aDoFEMaj,
                      std::vector<std::vector<std::pair<unsigned int, unsigned int> > >& aFeMajFrac);
 
-  bool fillFEErrors(const sistrip::FEDBuffer* aBuffer,
+  bool fillFEErrors(const sistrip::FEDBuffer& aBuffer,
                     const bool aDoFEMaj,
                     std::vector<std::vector<std::pair<unsigned int, unsigned int> > >& aFeMajFrac);
 
-  bool fillChannelErrors(const sistrip::FEDBuffer* aBuffer,
+  bool fillChannelErrors(const sistrip::FEDBuffer& aBuffer,
                          bool& aFullDebug,
                          const unsigned int aPrintDebug,
                          unsigned int& aCounterMonitoring,

diff --git a/DQM/SiStripMonitorHardware/interface/SiStripFEDSpyBuffer.h b/DQM/SiStripMonitorHardware/interface/SiStripFEDSpyBuffer.h
@@ -32,8 +32,16 @@ namespace sistrip {
   //class representing spy channel buffers
   class FEDSpyBuffer : public FEDBufferBase {
   public:
+    /**
+     * constructor from a FEDRawData buffer
+     *
+     * The sistrip::preconstructCheckFEDSpyBuffer() method should be used
+     * to check the validity of fedBuffer before constructing a sistrip::FEDBuffer.
+     *
+     * @see sistrip::preconstructCheckFEDSpyBuffer()
+     */
     //construct from buffer
-    FEDSpyBuffer(const uint8_t* fedBuffer, const size_t fedBufferSize);
+    explicit FEDSpyBuffer(const FEDRawData& fedBuffer);
     ~FEDSpyBuffer() override;
     void print(std::ostream& os) const override;
 
@@ -85,6 +93,28 @@ namespace sistrip {
   // Inline function definitions
   //
 
+  /**
+   * Check if a FEDRawData object satisfies the requirements for constructing a sistrip::FEDSpyBuffer
+   *
+   * These are:
+   *   - those from sistrip::preconstructCheckFEDBufferBase() (with checkRecognizedFormat equal to true)
+   *   - the readout mode should be equal to sistrip::READOUT_MODE_SPY
+   *
+   * In case any check fails, a value different from sistrip::FEDBufferStatusCode::SUCCESS
+   * is returned, and detailed information printed to LogDebug("FEDBuffer"), if relevant.
+   *
+   * @see sistrip::preconstructCheckFEDBufferBase()
+   */
+  inline FEDBufferStatusCode preconstructCheckFEDSpyBuffer(const FEDRawData& fedBuffer) {
+    const auto st_base = preconstructCheckFEDBufferBase(fedBuffer, true);
+    if (FEDBufferStatusCode::SUCCESS != st_base)
+      return st_base;
+    const TrackerSpecialHeader hdr{fedBuffer.data() + 8};
+    if (READOUT_MODE_SPY != hdr.readoutMode())
+      return FEDBufferStatusCode::EXPECT_SPY;
+    return FEDBufferStatusCode::SUCCESS;
+  }
+
   //FEDSpyChannelUnpacker
 
   inline FEDSpyChannelUnpacker::FEDSpyChannelUnpacker(const FEDChannel& channel)