Remove VLAs (#124)

Removing the use of VLAs in the client and receiver side code. In addition cleaning up sending jsonheader

slsDetectorSoftware/src/DetectorImpl.cpp

@@ -434,7 +434,8 @@ void DetectorImpl::readFrameFromReceiver() {
         numInterfaces = Parallel(&Module::getNumberofUDPInterfacesFromShm, {})
                             .squash(); // cannot pick up from zmq
     }
-    bool runningList[zmqSocket.size()], connectList[zmqSocket.size()];
+    std::vector<bool> runningList(zmqSocket.size());
+    std::vector<bool> connectList(zmqSocket.size());
     int numRunning = 0;
     for (size_t i = 0; i < zmqSocket.size(); ++i) {
         if (zmqSocket[i]->Connect() == 0) {

slsDetectorSoftware/src/Module.cpp

@@ -458,12 +458,11 @@ std::vector<uint64_t> Module::getNumMissingPackets() const {
             throw RuntimeError("Receiver " + std::to_string(moduleId) +
                                " returned error: " + std::string(mess));
         } else {
-            int nports = -1;
+            int nports = 0;
             client.Receive(&nports, sizeof(nports));
-            uint64_t mp[nports];
+            std::vector<uint64_t> retval(nports);
-            memset(mp, 0, sizeof(mp));
+            client.Receive(retval.data(),
-            client.Receive(mp, sizeof(mp));
+                           sizeof(decltype(retval[0])) * retval.size());
-            std::vector<uint64_t> retval(mp, mp + nports);
             LOG(logDEBUG1) << "Missing packets of Receiver" << moduleId << ": "
                            << sls::ToString(retval);
             return retval;
@@ -1679,27 +1678,23 @@ void Module::getBadChannels(const std::string &fname) const {
     }
     // receive badchannels
     int nch = -1;
-    std::vector<int> badchannels;
     client.Receive(&nch, sizeof(nch));
+    std::vector<int> badchannels(nch);
     if (nch > 0) {
-        int temp[nch];
+        client.Receive(badchannels.data(),
-        memset(temp, 0, sizeof(temp));
+                       sizeof(badchannels[0]) * badchannels.size());
-        client.Receive(temp, sizeof(temp));
+        for (size_t i = 0; i < badchannels.size(); ++i) {
-        badchannels.insert(badchannels.end(), &temp[0], &temp[nch]);
-        for (int i = 0; i < (int)badchannels.size(); ++i) {
             LOG(logDEBUG1) << i << ":" << badchannels[i];
         }
     }
 
     // save to file
-    std::ofstream outfile;
+    std::ofstream outfile(fname);
-    outfile.open(fname.c_str(), std::ios_base::out);
     if (!outfile.is_open()) {
         throw RuntimeError("Could not create file to save bad channels");
     }
-    for (int i = 0; i < nch; ++i) {
+    for (auto ch : badchannels)
-        outfile << badchannels[i] << '\n';
+        outfile << ch << '\n';
-    }
     LOG(logDEBUG1) << nch << " bad channels saved to file";
 }
 
@@ -1712,10 +1707,9 @@ void Module::setBadChannels(const std::string &fname) {
     }
     std::vector<int> badchannels;
     for (std::string line; std::getline(input_file, line);) {
-        if (line.find(' ') != std::string::npos) {
+        line.erase(std::remove_if(begin(line), end(line), isspace),
-            line.erase(line.find(' '));
+                   end(line)); // remove space
-        }
+        if (!line.empty()) {
-        if (line.length() >= 1) {
             std::istringstream iss(line);
             int ival = 0;
             iss >> ival;
@@ -2139,30 +2133,31 @@ void Module::startPattern() { sendToDetector(F_START_PATTERN); }
 // Moench
 
 std::map<std::string, std::string> Module::getAdditionalJsonHeader() const {
+    //TODO, refactor this function with a more robust sending. 
+    // Now assuming whitespace separated key value
     if (!shm()->useReceiverFlag) {
         throw RuntimeError("Set rx_hostname first to use receiver parameters "
                            "(zmq json header)");
     }
-    int fnum = F_GET_ADDITIONAL_JSON_HEADER;
-    int ret = FAIL;
-    int size = 0;
     auto client = ReceiverSocket(shm()->rxHostname, shm()->rxTCPPort);
-    client.Send(&fnum, sizeof(fnum));
+    client.Send(F_GET_ADDITIONAL_JSON_HEADER);
-    client.Receive(&ret, sizeof(ret));
+    auto ret = client.Receive<int>();
     if (ret == FAIL) {
         char mess[MAX_STR_LENGTH]{};
         client.Receive(mess, MAX_STR_LENGTH);
         throw RuntimeError("Receiver " + std::to_string(moduleId) +
                            " returned error: " + std::string(mess));
     } else {
-        client.Receive(&size, sizeof(size));
+        auto size = client.Receive<int>();
+        std::string buff(size, '\0');
         std::map<std::string, std::string> retval;
         if (size > 0) {
-            char retvals[size * 2][SHORT_STR_LENGTH];
+            client.Receive(&buff[0], buff.size());
-            memset(retvals, 0, sizeof(retvals));
+            std::istringstream iss(buff);
-            client.Receive(retvals, sizeof(retvals));
+            std::string key, value;
-            for (int i = 0; i < size; ++i) {
+            while(iss >> key){
-                retval[retvals[2 * i]] = retvals[2 * i + 1];
+                iss >> value;
+                retval[key] = value;
             }
         }
         LOG(logDEBUG) << "Getting additional json header " << ToString(retval);
@@ -2185,26 +2180,20 @@ void Module::setAdditionalJsonHeader(
                 "Key cannot be empty. Both can have max 20 characters");
         }
     }
-    const int size = jsonHeader.size();
+    std::ostringstream oss;
-    int fnum = F_SET_ADDITIONAL_JSON_HEADER;
+    for (auto& it : jsonHeader)
-    int ret = FAIL;
+        oss << it.first << ' ' << it.second << ' ';
+    auto buff = oss.str();
+    const auto size = static_cast<int>(buff.size());
     LOG(logDEBUG) << "Sending to receiver additional json header "
                   << ToString(jsonHeader);
     auto client = ReceiverSocket(shm()->rxHostname, shm()->rxTCPPort);
-    client.Send(&fnum, sizeof(fnum));
+    client.Send(F_SET_ADDITIONAL_JSON_HEADER);
-    client.Send(&size, sizeof(size));
+    client.Send(size);
-    if (size > 0) {
+    if (size > 0)
-        char args[size * 2][SHORT_STR_LENGTH];
+        client.Send(&buff[0], buff.size());
-        memset(args, 0, sizeof(args));
+
-        int iarg = 0;
+    auto ret = client.Receive<int>();
-        for (auto &it : jsonHeader) {
-            sls::strcpy_safe(args[iarg], it.first.c_str());
-            sls::strcpy_safe(args[iarg + 1], it.second.c_str());
-            iarg += 2;
-        }
-        client.Send(args, sizeof(args));
-    }
-    client.Receive(&ret, sizeof(ret));
     if (ret == FAIL) {
         char mess[MAX_STR_LENGTH]{};
         client.Receive(mess, MAX_STR_LENGTH);

slsReceiverSoftware/src/BinaryFile.cpp

@@ -19,9 +19,7 @@ BinaryFile::BinaryFile(int ind, uint32_t *maxf, int *nd, std::string *fname,
                        int *dindex, int *nunits, uint64_t *nf, uint32_t *dr,
                        uint32_t *portno, bool *smode)
     : File(ind, BINARY, maxf, nd, fname, fpath, findex, owenable, dindex,
-           nunits, nf, dr, portno, smode),
+           nunits, nf, dr, portno, smode) {
-      filefd(nullptr), numFramesInFile(0), numActualPacketsInFile(0),
-      maxMasterFileSize(2000) {
 #ifdef VERBOSE
     PrintMembers();
 #endif
@@ -239,6 +237,8 @@ void BinaryFile::CreateMasterFile(bool masterFileWriteEnable,
                 (long long int)masterFileAttributes.gateDelay2Ns,
                 (long long int)masterFileAttributes.gateDelay3Ns,
                 masterFileAttributes.gates, ctime(&t));
+
+        //TODO! snprintf? This would already have been a buffer overflow
         if (strlen(message) > maxMasterFileSize) {
             throw sls::RuntimeError("Master File Size " +
                                     std::to_string(strlen(message)) +

slsReceiverSoftware/src/BinaryFile.h

@@ -51,9 +51,12 @@ class BinaryFile : private virtual slsDetectorDefs, public File {
   private:
     int WriteData(char *buf, int bsize);
 
-    FILE *filefd;
+    FILE *filefd = nullptr;
     static FILE *masterfd;
-    uint32_t numFramesInFile;
+    uint32_t numFramesInFile = 0;
-    uint64_t numActualPacketsInFile;
+    uint64_t numActualPacketsInFile = 0;
-    const size_t maxMasterFileSize;
+    //Make sure this is known at compile time
-};
+    //TODO! Later away from stack allocation of message
+    static constexpr size_t maxMasterFileSize = 2000; 
+    
+};

slsReceiverSoftware/src/ClientInterface.cpp

@@ -901,14 +901,12 @@ int ClientInterface::get_frame_index(Interface &socket) {
 }
 
 int ClientInterface::get_missing_packets(Interface &socket) {
-    std::vector<uint64_t> m = impl()->getNumMissingPackets();
+    auto missing_packets = impl()->getNumMissingPackets();
-    LOG(logDEBUG1) << "missing packets:" << sls::ToString(m);
+    LOG(logDEBUG1) << "missing packets:" << sls::ToString(missing_packets);
-    int retvalsize = m.size();
+    auto size = static_cast<int>(missing_packets.size());
-    uint64_t retval[retvalsize];
-    std::copy(std::begin(m), std::end(m), retval);
     socket.Send(OK);
-    socket.Send(&retvalsize, sizeof(retvalsize));
+    socket.Send(size);
-    socket.Send(retval, sizeof(retval));
+    socket.Send(missing_packets.data(), sizeof(missing_packets[0])* missing_packets.size());
     return OK;
 }
 
@@ -1200,13 +1198,15 @@ int ClientInterface::restream_stop(Interface &socket) {
 
 int ClientInterface::set_additional_json_header(Interface &socket) {
     std::map<std::string, std::string> json;
-    int size = socket.Receive<int>();
+    auto size = socket.Receive<int>();
     if (size > 0) {
-        char args[size * 2][SHORT_STR_LENGTH];
+        std::string buff(size, '\0');
-        memset(args, 0, sizeof(args));
+        socket.Receive(&buff[0], buff.size());
-        socket.Receive(args, sizeof(args));
+        std::istringstream iss(buff);
-        for (int i = 0; i < size; ++i) {
+        std::string key, value;
-            json[args[2 * i]] = args[2 * i + 1];
+        while(iss >> key){
+            iss >> value;
+            json[key] = value;
         }
     }
     verifyIdle(socket);
@@ -1218,18 +1218,15 @@ int ClientInterface::set_additional_json_header(Interface &socket) {
 int ClientInterface::get_additional_json_header(Interface &socket) {
     std::map<std::string, std::string> json = impl()->getAdditionalJsonHeader();
     LOG(logDEBUG1) << "additional json header:" << sls::ToString(json);
-    int size = json.size();
+    std::ostringstream oss;
+    for (auto & it : json){
+        oss << it.first << ' ' << it.second << ' ';
+    }
+    auto buff = oss.str();
+    auto size = static_cast<int>(buff.size());
     socket.sendResult(size);
     if (size > 0) {
-        char retvals[size * 2][SHORT_STR_LENGTH];
+        socket.Send(&buff[0], buff.size());
-        memset(retvals, 0, sizeof(retvals));
-        int iarg = 0;
-        for (auto &it : json) {
-            sls::strcpy_safe(retvals[iarg], it.first.c_str());
-            sls::strcpy_safe(retvals[iarg + 1], it.second.c_str());
-            iarg += 2;
-        }
-        socket.Send(retvals, sizeof(retvals));
     }
     return OK;
 }

slsReceiverSoftware/src/DataProcessor.cpp

@@ -411,6 +411,7 @@ void DataProcessor::PadMissingPackets(char *buf) {
 
 /** ctb specific */
 void DataProcessor::RearrangeDbitData(char *buf) {
+    //TODO! (Erik) Refactor and add tests
     int totalSize = (int)(*((uint32_t *)buf));
     int ctbDigitalDataBytes =
         totalSize - (*ctbAnalogDataBytes) - (*ctbDbitOffset);
@@ -429,9 +430,8 @@ void DataProcessor::RearrangeDbitData(char *buf) {
     // ceil as numResult8Bits could be decimal
     const int numResult8Bits =
         ceil((double)(numSamples * (*ctbDbitList).size()) / 8.00);
-    uint8_t result[numResult8Bits];
+    std::vector<uint8_t> result(numResult8Bits);
-    memset(result, 0, numResult8Bits * sizeof(uint8_t));
+    uint8_t *dest = &result[0];
-    uint8_t *dest = result;
 
     auto *source = (uint64_t *)(buf + digOffset + (*ctbDbitOffset));
 
@@ -459,6 +459,6 @@ void DataProcessor::RearrangeDbitData(char *buf) {
     }
 
     // copy back to buf and update size
-    memcpy(buf + digOffset, result, numResult8Bits * sizeof(uint8_t));
+    memcpy(buf + digOffset, result.data(), numResult8Bits * sizeof(uint8_t));
     (*((uint32_t *)buf)) = numResult8Bits * sizeof(uint8_t);
 }