Blob Blame History Raw
From 092d06fd403331bf082dd0d2d56718a9ee31efec Mon Sep 17 00:00:00 2001
From: Mattias Ellert <mattias.ellert@physics.uu.se>
Date: Thu, 15 Feb 2018 18:12:41 +0100
Subject: [PATCH 1/4] Don't call front() on empty containers

---
 math/mathcore/inc/Fit/FitData.h               |  4 ++--
 math/mathcore/inc/Math/WrappedParamFunction.h | 10 +++++-----
 math/mathcore/src/BinData.cxx                 | 24 ++++++++++++------------
 math/mathcore/src/FitData.cxx                 |  2 +-
 4 files changed, 20 insertions(+), 20 deletions(-)

diff --git a/math/mathcore/inc/Fit/FitData.h b/math/mathcore/inc/Fit/FitData.h
index cb8fa2f0be3..9579bde92bf 100644
--- a/math/mathcore/inc/Fit/FitData.h
+++ b/math/mathcore/inc/Fit/FitData.h
@@ -185,7 +185,7 @@ namespace ROOT {
 
             for (unsigned int i = 0; i < fDim; i++) {
                fCoords[i].resize(fMaxPoints + VectorPadding(fMaxPoints));
-               fCoordsPtr[i] = &fCoords[i].front();
+               fCoordsPtr[i] = fCoords[i].empty() ? NULL : &fCoords[i].front();
             }
 
             if (fpTmpCoordVector) {
@@ -354,7 +354,7 @@ namespace ROOT {
                unsigned padding = VectorPadding(fNPoints);
                fCoords[i].resize(fNPoints + padding);
                std::copy(fCoordsPtr[i], fCoordsPtr[i] + fNPoints + padding, fCoords[i].begin());
-               fCoordsPtr[i] = &fCoords[i].front();
+               fCoordsPtr[i] = fCoords[i].empty() ? NULL : &fCoords[i].front();
             }
 
             fWrapped = false;
diff --git a/math/mathcore/inc/Math/WrappedParamFunction.h b/math/mathcore/inc/Math/WrappedParamFunction.h
index 38b9af7639f..0c860672f9e 100644
--- a/math/mathcore/inc/Math/WrappedParamFunction.h
+++ b/math/mathcore/inc/Math/WrappedParamFunction.h
@@ -95,7 +95,7 @@ class WrappedParamFunction : public IParamMultiFunction {
    }
 
    const double * Parameters() const {
-      return  &(fParams.front());
+      return fParams.empty() ? NULL : &fParams.front();
    }
 
    void SetParameters(const double * p)  {
@@ -172,7 +172,7 @@ class WrappedParamFunctionGen : public IParamMultiFunction {
 
    /// clone the function
    IMultiGenFunction * Clone() const {
-      return new WrappedParamFunctionGen(fFunc, fDim, fParams.size() , &fParams.front(), &fParIndices.front());
+      return new WrappedParamFunctionGen(fFunc, fDim, fParams.size(), fParams.empty() ? NULL : &fParams.front(), fParIndices.empty() ? NULL : &fParIndices.front());
    }
 
 private:
@@ -183,7 +183,7 @@ class WrappedParamFunctionGen : public IParamMultiFunction {
 public:
 
    const double * Parameters() const {
-      return  &(fParams.front());
+      return fParams.empty() ? NULL : &fParams.front();
    }
 
    void SetParameters(const double * p)  {
@@ -227,7 +227,7 @@ class WrappedParamFunctionGen : public IParamMultiFunction {
 //       std::copy(fX.begin(), fX.end(), oi);
 //       std::cout << std::endl;
 
-      return (*fFunc)( &fX.front() );
+      return (*fFunc)( fX.empty() ? NULL : &fX.front() );
    }
 
 
@@ -264,7 +264,7 @@ class WrappedParamFunctionGen : public IParamMultiFunction {
 //       std::cout << this << std::endl;
 
       // set parameter values in fX
-      SetParValues(npar, &fParams.front() );
+      SetParValues(npar, fParams.empty() ? NULL : &fParams.front());
       for (unsigned int i = 0; i < npar; ++i) {
          unsigned int j = fParIndices[i];
          assert ( j  < npar + fDim);
diff --git a/math/mathcore/src/BinData.cxx b/math/mathcore/src/BinData.cxx
index 16c9e375634..50a47169272 100644
--- a/math/mathcore/src/BinData.cxx
+++ b/math/mathcore/src/BinData.cxx
@@ -284,7 +284,7 @@ namespace ROOT {
 
           for ( unsigned int i=0; i<fDim; i++ )
           {
-            fCoordErrorsPtr[i] = &fCoordErrors[i].front();
+            fCoordErrorsPtr[i] = fCoordErrors[i].empty() ? NULL : &fCoordErrors[i].front();
           }
         }
 
@@ -371,7 +371,7 @@ namespace ROOT {
       if ( kNoError == fErrorType )
       {
          fDataError.resize(fNPoints + FitData::VectorPadding(fNPoints));
-         fDataErrorPtr = &fDataError.front();
+         fDataErrorPtr = fDataError.empty() ? NULL : &fDataError.front();
       }
 
       for ( unsigned int i=0; i < fNPoints; i++ )
@@ -659,7 +659,7 @@ namespace ROOT {
     void BinData::InitDataVector ()
     {
        fData.resize(fMaxPoints + FitData::VectorPadding(fMaxPoints));
-       fDataPtr = &fData.front();
+       fDataPtr = fData.empty() ? NULL : &fData.front();
     }
 
     void BinData::InitializeErrors()
@@ -698,7 +698,7 @@ namespace ROOT {
         {
            fCoordErrors[i].resize(fMaxPoints + FitData::VectorPadding(fMaxPoints));
 
-           fCoordErrorsPtr[i] = &fCoordErrors[i].front();
+           fCoordErrorsPtr[i] = fCoordErrors[i].empty() ? NULL : &fCoordErrors[i].front();
         }
 
         fpTmpCoordErrorVector = new double[fDim];
@@ -712,7 +712,7 @@ namespace ROOT {
       if ( kValueError == fErrorType || kCoordError == fErrorType )
       {
          fDataError.resize(fMaxPoints + FitData::VectorPadding(fMaxPoints));
-         fDataErrorPtr = &fDataError.front();
+         fDataErrorPtr = fDataError.empty() ? NULL : &fDataError.front();
 
          fDataErrorHigh.clear();
          fDataErrorHighPtr = NULL;
@@ -722,10 +722,10 @@ namespace ROOT {
       else if ( fErrorType == kAsymError )
       {
          fDataErrorHigh.resize(fMaxPoints + FitData::VectorPadding(fMaxPoints));
-         fDataErrorHighPtr = &fDataErrorHigh.front();
+         fDataErrorHighPtr = fDataErrorHigh.empty() ? NULL : &fDataErrorHigh.front();
 
          fDataErrorLow.resize(fMaxPoints + FitData::VectorPadding(fMaxPoints));
-         fDataErrorLowPtr = &fDataErrorLow.front();
+         fDataErrorLowPtr = fDataErrorLow.empty() ? NULL : &fDataErrorLow.front();
 
          fDataError.clear();
          fDataErrorPtr = NULL;
@@ -770,7 +770,7 @@ namespace ROOT {
       unsigned vectorPadding = FitData::VectorPadding(fNPoints);
       fData.resize(fNPoints + vectorPadding);
       std::copy( fDataPtr, fDataPtr + fNPoints, fData.begin() );
-      fDataPtr = &fData.front();
+      fDataPtr = fData.empty() ? NULL : &fData.front();
 
       for ( unsigned int i=0; i < fDim; i++ )
       {
@@ -785,7 +785,7 @@ namespace ROOT {
 
         fDataError.resize(fNPoints + vectorPadding);
         std::copy(fDataErrorPtr, fDataErrorPtr + fNPoints + vectorPadding, fDataError.begin());
-        fDataErrorPtr = &fDataError.front();
+        fDataErrorPtr = fDataError.empty() ? NULL : &fDataError.front();
       }
 
       if ( kValueError == fErrorType )
@@ -804,7 +804,7 @@ namespace ROOT {
           assert( fCoordErrorsPtr[i] );
           fCoordErrors[i].resize(fNPoints + vectorPadding);
           std::copy(fCoordErrorsPtr[i], fCoordErrorsPtr[i] + fNPoints + vectorPadding, fCoordErrors[i].begin());
-          fCoordErrorsPtr[i] = &fCoordErrors[i].front();
+          fCoordErrorsPtr[i] = fCoordErrors[i].empty() ? NULL : &fCoordErrors[i].front();
         }
 
         if( kAsymError == fErrorType )
@@ -817,8 +817,8 @@ namespace ROOT {
           fDataErrorLow.resize(fNPoints + vectorPadding);
           std::copy(fDataErrorHighPtr, fDataErrorHighPtr + fNPoints + vectorPadding, fDataErrorHigh.begin());
           std::copy(fDataErrorLowPtr, fDataErrorLowPtr + fNPoints + vectorPadding, fDataErrorLow.begin());
-          fDataErrorHighPtr = &fDataErrorHigh.front();
-          fDataErrorLowPtr = &fDataErrorLow.front();
+          fDataErrorHighPtr = fDataErrorHigh.empty() ? NULL : &fDataErrorHigh.front();
+          fDataErrorLowPtr = fDataErrorLow.empty() ? NULL : &fDataErrorLow.front();
         }
       }
 
diff --git a/math/mathcore/src/FitData.cxx b/math/mathcore/src/FitData.cxx
index 76c792dbe66..0b62e607534 100644
--- a/math/mathcore/src/FitData.cxx
+++ b/math/mathcore/src/FitData.cxx
@@ -231,7 +231,7 @@ namespace ROOT {
             fCoordsPtr.resize(fDim);
 
             for (unsigned int i = 0; i < fDim; i++) {
-               fCoordsPtr[i] = &fCoords[i].front();
+               fCoordsPtr[i] = fCoords[i].empty() ? NULL : &fCoords[i].front();
             }
          }
 

From 711cb8598110b774ae7c206c0730b907e7650e2e Mon Sep 17 00:00:00 2001
From: Mattias Ellert <mattias.ellert@physics.uu.se>
Date: Thu, 15 Feb 2018 18:18:35 +0100
Subject: [PATCH 2/4] Call resize() when the size should be changed, reserve()
 does not change the size

---
 math/mathcore/src/TKDTreeBinning.cxx | 2 +-
 tree/tree/test/TBasket.cxx           | 8 ++++----
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/math/mathcore/src/TKDTreeBinning.cxx b/math/mathcore/src/TKDTreeBinning.cxx
index 09647958dc7..d0b06e1a99f 100644
--- a/math/mathcore/src/TKDTreeBinning.cxx
+++ b/math/mathcore/src/TKDTreeBinning.cxx
@@ -241,7 +241,7 @@ void TKDTreeBinning::SetTreeData() {
 
 void TKDTreeBinning::SetBinsContent() {
    // Sets the bins' content
-   fBinsContent.reserve(fNBins);
+   fBinsContent.resize(fNBins);
    for (UInt_t i = 0; i < fNBins; ++i)
       fBinsContent[i] = fDataBins->GetBucketSize();
    if ( fDataSize % fNBins != 0 )
diff --git a/tree/tree/test/TBasket.cxx b/tree/tree/test/TBasket.cxx
index fba69bd68d0..90eb330f7ee 100644
--- a/tree/tree/test/TBasket.cxx
+++ b/tree/tree/test/TBasket.cxx
@@ -102,7 +102,7 @@ TEST(TBasket, CreateAndDestroy)
    f->Close();
 
    Long64_t maxsize = f->GetSize();
-   memBuffer.reserve(maxsize);
+   memBuffer.resize(maxsize);
    f->CopyTo(&memBuffer[0], maxsize);
 
    delete f;
@@ -182,7 +182,7 @@ TEST(TBasket, TestUnsupportedIO)
 
    std::vector<char> memBuffer;
    Long64_t maxsize = f->GetSize();
-   memBuffer.reserve(maxsize);
+   memBuffer.resize(maxsize);
    f->CopyTo(&memBuffer[0], maxsize);
 
    TMemFile f2("tbasket_test.root", &memBuffer[0], maxsize, "READ");
@@ -230,7 +230,7 @@ TEST(TBasket, TestVarLengthArrays)
    f->Close();
    std::vector<char> memBuffer;
    Long64_t maxsize = f->GetSize();
-   memBuffer.reserve(maxsize);
+   memBuffer.resize(maxsize);
    f->CopyTo(&memBuffer[0], maxsize);
 
    TMemFile f2("tbasket_test.root", &memBuffer[0], maxsize, "READ");
@@ -334,7 +334,7 @@ TEST(TBasket, TestSettingIOBits)
    f->Close();
    std::vector<char> memBuffer;
    Long64_t maxsize = f->GetSize();
-   memBuffer.reserve(maxsize);
+   memBuffer.resize(maxsize);
    f->CopyTo(&memBuffer[0], maxsize);
 
    TMemFile f2("tbasket_test.root", &memBuffer[0], maxsize, "READ");

From 8dfcb54ca0457c2e8ae6102bb4a60ac60bd49004 Mon Sep 17 00:00:00 2001
From: Mattias Ellert <mattias.ellert@physics.uu.se>
Date: Thu, 15 Feb 2018 18:25:39 +0100
Subject: [PATCH 3/4] operator[] with an argument that is out of range causes
 undefined behavior

---
 math/mathcore/test/binarySearchTime.cxx | 5 ++---
 math/mathmore/test/testPermute.cxx      | 4 ++--
 2 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/math/mathcore/test/binarySearchTime.cxx b/math/mathcore/test/binarySearchTime.cxx
index c98017a0c55..0a611486489 100644
--- a/math/mathcore/test/binarySearchTime.cxx
+++ b/math/mathcore/test/binarySearchTime.cxx
@@ -58,9 +58,8 @@ template <typename T> bool testBinarySearch(const int n, double* tTMath, double*
    t.Start();
    for (int j = 0; j < npass; ++j) {
       for ( T elem = 0; elem < maxint; ++elem ) {
-         T* pind;
-         pind = std::lower_bound(&k[0], &k[n], elem);
-         Long_t index2 = (((pind != &(k[n])) && (*pind == elem)) ? (pind - &k[0]): ( pind - &k[0] - 1));
+         auto pind = std::lower_bound(k.begin(), k.end(), elem);
+         Long_t index2 = (((pind != k.end()) && (*pind == elem)) ? (pind - k.begin()) : (pind - k.begin() - 1));
          s2+= index2;
       }
    }
diff --git a/math/mathmore/test/testPermute.cxx b/math/mathmore/test/testPermute.cxx
index b75609b9958..f2a49da2e4d 100644
--- a/math/mathmore/test/testPermute.cxx
+++ b/math/mathmore/test/testPermute.cxx
@@ -64,7 +64,7 @@ bool checkPermute()
    //cout << original << vM << vS << endl;
 
    while ( TMath::Permute(n, &vM[0]) ) {
-      std::next_permutation(&vS[0], &vS[n]);
+      std::next_permutation(vS.begin(), vS.end());
       //cout << vM << vS << endl;
       equals &= equal(vM.begin(), vM.end(), vS.begin());
    }
@@ -99,7 +99,7 @@ void permuteTime(const int n, double* tTMath, double* tStd)
    t.Start();
    for (int j = 0; j < npass; ++j) {
       copy(original.begin(), original.end(), v.begin());
-      while ( std::next_permutation(&v[0], &v[n]) ) {}
+      while ( std::next_permutation(v.begin(), v.end()) ) {}
    }
    t.Stop();
    *tStd = t.RealTime();

From 80ba399340167f62368685e0bf03f9cb9a595954 Mon Sep 17 00:00:00 2001
From: Mattias Ellert <mattias.ellert@physics.uu.se>
Date: Thu, 15 Feb 2018 18:27:23 +0100
Subject: [PATCH 4/4] cut-and-paste errors, I guess...

---
 math/mathcore/test/testkdTreeBinning.cxx | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/math/mathcore/test/testkdTreeBinning.cxx b/math/mathcore/test/testkdTreeBinning.cxx
index 233b083ee5f..193ff1bb3e2 100644
--- a/math/mathcore/test/testkdTreeBinning.cxx
+++ b/math/mathcore/test/testkdTreeBinning.cxx
@@ -82,9 +82,9 @@ void testkdTreeBinning() {
    int ibinMax = kdBins->GetBinMaxDensity();
 
    std::cout << "Bin with minimum density: " << ibinMin << " density = " <<  kdBins->GetBinDensity(ibinMin) << " content = " << kdBins->GetBinContent(ibinMin)  << std::endl;
-   std::cout << "Bin with maximum density: " << ibinMax << " density = " <<  kdBins->GetBinDensity(ibinMax) << " content = " << kdBins->GetBinContent(ibinMin) << std::endl;
+   std::cout << "Bin with maximum density: " << ibinMax << " density = " <<  kdBins->GetBinDensity(ibinMax) << " content = " << kdBins->GetBinContent(ibinMax) << std::endl;
 
-   if (kdBins->GetBinDensity(ibinMax) != DATASZ/NBINS) 
+   if (kdBins->GetBinContent(ibinMax) != DATASZ/NBINS)
       Error("testkdTreeBinning","Wrong bin content");
 
    // order bins by density