Merge branch 'refactor' of github.com:slsdetectorgroup/slsDetectorPackage into refactor

slsDetectorSoftware/multiSlsDetector/multiSlsDetector.cpp

@@ -27,12 +27,12 @@
 #include <vector>
 
 multiSlsDetector::multiSlsDetector(int id, bool verify, bool update)
-    : detId(id), sharedMemory(0), thisMultiDetector(0),
+    : detId(id), sharedMemory(nullptr), thisMultiDetector(nullptr),
       client_downstream(false), totalProgress(0), progressIndex(0),
-      jointhread(0), acquiringDone(0), fdata(0), thisData(0),
+      jointhread(0), acquiringDone(0), fdata(nullptr), thisData(nullptr),
-      acquisition_finished(0), acqFinished_p(0), measurement_finished(0),
+      acquisition_finished(nullptr), acqFinished_p(nullptr), measurement_finished(nullptr),
-      measFinished_p(0), progress_call(0), pProgressCallArg(0), dataReady(0),
+      measFinished_p(nullptr), progress_call(nullptr), pProgressCallArg(nullptr), dataReady(nullptr),
-      pCallbackArg(0) {
+      pCallbackArg(nullptr) {
     setupMultiDetector(verify, update);
 }
 
@@ -274,11 +274,11 @@ void multiSlsDetector::freeSharedMemory(int detPos) {
     if (sharedMemory) {
         if (thisMultiDetector) {
             sharedMemory->UnmapSharedMemory(thisMultiDetector);
-            thisMultiDetector = 0;
+            thisMultiDetector = nullptr;
         }
         sharedMemory->RemoveSharedMemory();
         delete sharedMemory;
-        sharedMemory = 0;
+        sharedMemory = nullptr;
     }
 
     // zmq
@@ -334,11 +334,11 @@ void multiSlsDetector::initSharedMemory(bool verify) {
             // unmap
             if (thisMultiDetector) {
                 sharedMemory->UnmapSharedMemory(thisMultiDetector);
-                thisMultiDetector = 0;
+                thisMultiDetector = nullptr;
             }
             // delete
             delete sharedMemory;
-            sharedMemory = 0;
+            sharedMemory = nullptr;
         }
         throw;
     }
@@ -360,8 +360,8 @@ void multiSlsDetector::initializeDetectorStructure() {
     thisMultiDetector->numberOfChannelInclGapPixels[Y] = 0;
     thisMultiDetector->maxNumberOfChannelsPerDetector[X] = 0;
     thisMultiDetector->maxNumberOfChannelsPerDetector[Y] = 0;
-    for (int i = 0; i < MAX_TIMERS; ++i) {
+    for (long & i : thisMultiDetector->timerValue) {
-        thisMultiDetector->timerValue[i] = 0;
+        i = 0;
     }
 
     thisMultiDetector->acquiringFlag = false;
@@ -410,7 +410,7 @@ std::string multiSlsDetector::exec(const char *cmd) {
         throw std::exception();
     try {
         while (!feof(pipe)) {
-            if (fgets(buffer, bufsize, pipe) != NULL)
+            if (fgets(buffer, bufsize, pipe) != nullptr)
                 result += buffer;
         }
     } catch (...) {
@@ -854,8 +854,8 @@ int multiSlsDetector::writeConfigurationFile(const std::string &fname) {
                                             "threaded"};
 
     char *args[100];
-    for (int ia = 0; ia < 100; ++ia) {
+    for (auto & arg : args) {
-        args[ia] = new char[1000];
+        arg = new char[1000];
     }
     int ret = OK, ret1 = OK;
     std::ofstream outfile;
@@ -906,8 +906,8 @@ int multiSlsDetector::writeConfigurationFile(const std::string &fname) {
         ret = FAIL;
     }
 
-    for (int ia = 0; ia < 100; ++ia) {
+    for (auto & arg : args) {
-        delete[] args[ia];
+        delete[] arg;
     }
 
     return ret;
@@ -1775,7 +1775,7 @@ int multiSlsDetector::setROI(int n, ROI roiLimits[], int detPos) {
     for (int i = 0; i < ndet; ++i)
         nroi[i] = 0;
 
-    if ((n < 0) || (roiLimits == NULL))
+    if ((n < 0) || (roiLimits == nullptr))
         return FAIL;
 
     // ensures min < max
@@ -1893,7 +1893,7 @@ slsDetectorDefs::ROI *multiSlsDetector::getROI(int &n, int detPos) {
     ROI temproi;
     ROI roiLimits[maxroi];
     ROI *retval = new ROI[maxroi];
-    ROI *temp = 0;
+    ROI *temp = nullptr;
     int index = 0;
 
     // get each detector's roi array
@@ -1919,7 +1919,7 @@ slsDetectorDefs::ROI *multiSlsDetector::getROI(int &n, int detPos) {
 
     // empty roi
     if (!n)
-        return NULL;
+        return nullptr;
 
     FILE_LOG(logDEBUG1) << "ROI :" << std::endl;
     for (int j = 0; j < n; ++j) {
@@ -2684,9 +2684,9 @@ void multiSlsDetector::readFrameFromReceiver() {
     }
     int numConnected = numRunning;
     bool data = false;
-    char *image = NULL;
+    char *image = nullptr;
-    char *multiframe = NULL;
+    char *multiframe = nullptr;
-    char *multigappixels = NULL;
+    char *multigappixels = nullptr;
     int multisize = 0;
     // only first message header
     uint32_t size = 0, nPixelsX = 0, nPixelsY = 0, dynamicRange = 0;
@@ -2707,7 +2707,7 @@ void multiSlsDetector::readFrameFromReceiver() {
     while (running) {
         // reset data
         data = false;
-        if (multiframe != NULL)
+        if (multiframe != nullptr)
             memset(multiframe, 0xFF, multisize);
 
         // get each frame
@@ -2729,7 +2729,7 @@ void multiSlsDetector::readFrameFromReceiver() {
                     }
 
                     // if first message, allocate (all one time stuff)
-                    if (image == NULL) {
+                    if (image == nullptr) {
                         // allocate
                         size = doc["size"].GetUint();
                         multisize = size * zmqSocket.size();
@@ -2865,11 +2865,11 @@ void multiSlsDetector::readFrameFromReceiver() {
             zmqSocket[i]->Disconnect();
 
     // free resources
-    if (image != NULL)
+    if (image != nullptr)
         delete[] image;
-    if (multiframe != NULL)
+    if (multiframe != nullptr)
         delete[] multiframe;
-    if (multigappixels != NULL)
+    if (multigappixels != nullptr)
         delete[] multigappixels;
 }
 
@@ -2883,15 +2883,15 @@ int multiSlsDetector::processImageWithGapPixels(char *image, char *&gpImage) {
     int nychip = thisMultiDetector->numberOfDetector[Y] * 1;
 
     // allocate
-    if (gpImage == NULL)
+    if (gpImage == nullptr)
         gpImage = new char[gapdatabytes];
     // fill value
     memset(gpImage, 0xFF, gapdatabytes);
 
     const int b1chipx = 128;
     const int b1chipy = 256;
-    char *src = 0;
+    char *src = nullptr;
-    char *dst = 0;
+    char *dst = nullptr;
 
     // copying line by line
     src = image;
@@ -2951,7 +2951,7 @@ int multiSlsDetector::processImageWithGapPixels(char *image, char *&gpImage) {
     // horizontal filling
     {
         uint8_t temp, g1, g2;
-        char *dst_prevline = 0;
+        char *dst_prevline = nullptr;
         dst = gpImage;
         for (int row = 0; row < nychip; ++row) { // for each chip in a row
             dst += (b1chipy * nxb);
@@ -3190,7 +3190,6 @@ int multiSlsDetector::retrieveDetectorSetup(const std::string &fname1,
     char *args[10];
 
     char myargs[10][1000];
-    ;
 
     std::string sargname, sargval;
     int iline = 0;
@@ -3219,7 +3218,7 @@ int multiSlsDetector::retrieveDetectorSetup(const std::string &fname1,
                 while (ssstr.good()) {
                     ssstr >> sargname;
                     //  if (ssstr.good()) {
-                    strcpy(myargs[iargval], sargname.c_str());
+                    sls::strcpy_safe(myargs[iargval], sargname.c_str());
                     args[iargval] = myargs[iargval];
                     FILE_LOG(logDEBUG1) << args[iargval];
                     iargval++;
@@ -3346,9 +3345,9 @@ int multiSlsDetector::dumpDetectorSetup(const std::string &fname, int level) {
     outfile.open(outfname.c_str(), std::ios_base::out);
     if (outfile.is_open()) {
         auto cmd = slsDetectorCommand(this);
-        for (int iv = 0; iv < names.size(); ++iv) {
+        for (auto & name : names) {
-            sls::strcpy_safe(buffer, names[iv].c_str()); //this is...
+            sls::strcpy_safe(buffer, name.c_str()); //this is...
-            outfile << names[iv] << " " << cmd.executeLine(n_arguments, args, GET_ACTION)
+            outfile << name << " " << cmd.executeLine(n_arguments, args, GET_ACTION)
                     << std::endl;
         }
         outfile.close();
@@ -3613,7 +3612,7 @@ int multiSlsDetector::kbhit() {
     tv.tv_usec = 0;
     FD_ZERO(&fds);
     FD_SET(STDIN_FILENO, &fds); // STDIN_FILENO is 0
-    select(STDIN_FILENO + 1, &fds, NULL, NULL, &tv);
+    select(STDIN_FILENO + 1, &fds, nullptr, nullptr, &tv);
     return FD_ISSET(STDIN_FILENO, &fds);
 }
 

slsDetectorSoftware/multiSlsDetector/multiSlsDetector.h

@@ -994,7 +994,7 @@ class multiSlsDetector : public virtual slsDetectorDefs,
      * @param detPos -1 for all detectors in  list or specific detector position
      * @returns OK or FAIL
      */
-    int setROI(int n = -1, ROI roiLimits[] = NULL, int detPos = -1);
+    int setROI(int n = -1, ROI roiLimits[] = nullptr, int detPos = -1);
 
     /**
      * Get ROI from each detector and convert it to the multi detector scale
@@ -1075,7 +1075,7 @@ class multiSlsDetector : public virtual slsDetectorDefs,
      * @param detPos -1 for all detectors in  list or specific detector position
      * @returns number of trim energies
      */
-    int setTrimEn(int nen, int *en = NULL, int detPos = -1);
+    int setTrimEn(int nen, int *en = nullptr, int detPos = -1);
 
     /**
      * Returns the number of trim energies and their value  (Eiger)
@@ -1084,7 +1084,7 @@ class multiSlsDetector : public virtual slsDetectorDefs,
      * @param detPos -1 for all detectors in  list or specific detector position
      * @returns number of trim energies
      */
-    int getTrimEn(int *en = NULL, int detPos = -1);
+    int getTrimEn(int *en = nullptr, int detPos = -1);
 
     /**
      * Pulse Pixel (Eiger)

slsSupportLib/include/container_utils.h

@@ -100,40 +100,6 @@ T minusOneIfDifferent(const std::vector<T>& container)
 
 
 
-//TODO!(Erik)Should try to move away from using this in the slsDetectorPackage
-inline
-std::string concatenateIfDifferent(std::vector<std::string> container)
-{
-    if (allEqual(container)) {
-        return container.front();
-    } else {
-        std::string result;
-        for (const auto& s : container)
-            result += s + "+";
-        return result;
-    }
-}
-inline
-std::vector<std::string> split(const std::string& strToSplit, char delimeter)
-{
-    std::stringstream ss(strToSplit);
-    std::string item;
-    std::vector<std::string> splittedStrings;
-    while (std::getline(ss, item, delimeter)) {
-        splittedStrings.push_back(item);
-    }
-    return splittedStrings;
-}
-
-inline
-std::string concatenateNonEmptyStrings(const std::vector<std::string>& vec){
-    std::string ret;
-    for (const auto& s : vec)
-        if (!s.empty())
-            ret += s + "+";
-    return ret;
-}
-
 } // namespace sls
 
 #endif // CONTAINER_UTILS_H

slsSupportLib/include/string_utils.h

@@ -1,11 +1,16 @@
+#pragma once
+
+#include <string>
+#include <vector>
+
 namespace sls {
 
 /* Implementation of a safe string copy function for setting fields in 
 for example the multi sls detector. It tries to copy the size of the 
 destination from the source, stopping on '\0'. 
 
-Warning this would truncate the source string and should be used with care. 
+Warning this will truncate the source string and should be used with care. 
-Still this is better than strcpy...
+Still this is better than strcpy and a buffer overflow...
 */
 template <size_t array_size>
 void strcpy_safe(char (&destination)[array_size], const char *source) {
@@ -13,5 +18,21 @@ void strcpy_safe(char (&destination)[array_size], const char *source) {
     destination[array_size - 1] = '\0';
 }
 
+/* 
+Split a string using the specified delimeter and return a vector of strings.
+TODO! Look into switching to absl or a string_view based implementation. Current
+implementation should not be used in a performance critical place.
+*/
+std::vector<std::string> split(const std::string &strToSplit, char delimeter);
+
+/*
+Concatenate the non empty strings in the vector using +
+*/
+std::string concatenateNonEmptyStrings(const std::vector<std::string> &vec);
+
+/*
+Concatenate strings using + if the strings are different
+*/
+std::string concatenateIfDifferent(std::vector<std::string> container);
 
 }; // namespace sls

slsSupportLib/src/string_utils.cpp

@@ -1,9 +1,41 @@
 
-// #include <cstring>
+#include "string_utils.h"
-// namespace sls{
+#include "container_utils.h"
+#include <sstream>
+namespace sls{
 
-// void strcpy_safe(char *dst, const char *src, size_t size){
-//     strncpy(dst, src, size);
-// }
 
-// };
+std::vector<std::string> split(const std::string& strToSplit, char delimeter)
+{
+    std::stringstream ss(strToSplit);
+    std::string item;
+    std::vector<std::string> splittedStrings;
+    while (std::getline(ss, item, delimeter)) {
+        splittedStrings.push_back(item);
+    }
+    return splittedStrings;
+}
+
+
+std::string concatenateNonEmptyStrings(const std::vector<std::string>& vec){
+    std::string ret;
+    for (const auto& s : vec)
+        if (!s.empty())
+            ret += s + '+';
+    return ret;
+}
+
+std::string concatenateIfDifferent(std::vector<std::string> container)
+{
+    if (allEqual(container)) {
+        return container.front();
+    } else {
+        std::string result;
+        for (const auto& s : container)
+            result += s + '+';
+        return result;
+    }
+}
+
+
+}; // namespace sls

tests/src/test-container_utils.cpp

@@ -104,57 +104,3 @@ TEST_CASE("minus one does not have side effects"){
     REQUIRE(v[0]==1);
 }
 
-TEST_CASE("Concat") {
-  std::vector<std::string> v{"one", "one", "one"};
-  std::vector<std::string> v2{"one", "one", "one"};
-  auto r = sls::concatenateIfDifferent(v);
-  REQUIRE(r == std::string("one"));
-  r.clear();
-
-  // make sure we didn't modify the string
-  REQUIRE(v == v2);
-
-  SECTION("add a different value"){
-      v.push_back("two");
-      REQUIRE(v!=v2);
-      REQUIRE( sls::concatenateIfDifferent(v) == "one+one+one+two+");
-  }
-}
-
-TEST_CASE("split a string with end delimiter"){
-  std::string s("abra+kadabra+");
-  auto r =sls::split(s, '+');
-  REQUIRE(r.size()==2);
-  REQUIRE(r[0]=="abra");
-  REQUIRE(r[1]=="kadabra");
-}
-
-TEST_CASE("split a string without end delimiter"){
-  std::string s("abra+kadabra+filibom");
-  auto r =sls::split(s, '+');
-  REQUIRE(r.size()==3);
-  REQUIRE(r[0]=="abra");
-  REQUIRE(r[1]=="kadabra");
-  REQUIRE(r[2]=="filibom");
-}
-
-TEST_CASE("concatenate non empty strings"){
-  std::vector<std::string> vec{"hej", "kalas", "", "foto"};
-  REQUIRE(vec.size()==4);
-  auto ret = sls::concatenateNonEmptyStrings(vec);
-  REQUIRE(ret == "hej+kalas+foto+");
-}
-
-TEST_CASE("concatenate non empty strings with only emty"){
-  std::vector<std::string> vec{"", "", ""};
-  REQUIRE(vec.size()==3);
-  auto ret = sls::concatenateNonEmptyStrings(vec);
-  REQUIRE(ret.empty());
-}
-
-TEST_CASE("concatenate non empty strings with one element"){
-  std::vector<std::string> vec{"", "hej", "", "", ""};
-  REQUIRE(vec.size()==5);
-  auto ret = sls::concatenateNonEmptyStrings(vec);
-  REQUIRE(ret=="hej+");
-}

tests/src/test-string_utils.cpp

@@ -30,5 +30,61 @@ TEST_CASE("copy a long string"){
     sls::strcpy_safe(dst, src);
     REQUIRE(dst[0]=='s');
     REQUIRE(dst[1]=='o');
-    REQUIRE(dst[3]=='\0');
+    REQUIRE(dst[2]=='\0');
+    
+}
+
+TEST_CASE("Concat") {
+  std::vector<std::string> v{"one", "one", "one"};
+  std::vector<std::string> v2{"one", "one", "one"};
+  auto r = sls::concatenateIfDifferent(v);
+  REQUIRE(r == std::string("one"));
+  r.clear();
+
+  // make sure we didn't modify the string
+  REQUIRE(v == v2);
+
+  SECTION("add a different value"){
+      v.emplace_back("two");
+      REQUIRE(v!=v2);
+      REQUIRE( sls::concatenateIfDifferent(v) == "one+one+one+two+");
+  }
+}
+
+TEST_CASE("split a string with end delimiter"){
+  std::string s("abra+kadabra+");
+  auto r =sls::split(s, '+');
+  REQUIRE(r.size()==2);
+  REQUIRE(r[0]=="abra");
+  REQUIRE(r[1]=="kadabra");
+}
+
+TEST_CASE("split a string without end delimiter"){
+  std::string s("abra+kadabra+filibom");
+  auto r =sls::split(s, '+');
+  REQUIRE(r.size()==3);
+  REQUIRE(r[0]=="abra");
+  REQUIRE(r[1]=="kadabra");
+  REQUIRE(r[2]=="filibom");
+}
+
+TEST_CASE("concatenate non empty strings"){
+  std::vector<std::string> vec{"hej", "kalas", "", "foto"};
+  REQUIRE(vec.size()==4);
+  auto ret = sls::concatenateNonEmptyStrings(vec);
+  REQUIRE(ret == "hej+kalas+foto+");
+}
+
+TEST_CASE("concatenate non empty strings with only emty"){
+  std::vector<std::string> vec{"", "", ""};
+  REQUIRE(vec.size()==3);
+  auto ret = sls::concatenateNonEmptyStrings(vec);
+  REQUIRE(ret.empty());
+}
+
+TEST_CASE("concatenate non empty strings with one element"){
+  std::vector<std::string> vec{"", "hej", "", "", ""};
+  REQUIRE(vec.size()==5);
+  auto ret = sls::concatenateNonEmptyStrings(vec);
+  REQUIRE(ret=="hej+");
 }