diff --git a/slsDetectorSoftware/src/DetectorImpl.cpp b/slsDetectorSoftware/src/DetectorImpl.cpp index 1a9f244ad..95f7a7eb1 100644 --- a/slsDetectorSoftware/src/DetectorImpl.cpp +++ b/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 runningList(zmqSocket.size()); + std::vector connectList(zmqSocket.size()); int numRunning = 0; for (size_t i = 0; i < zmqSocket.size(); ++i) { if (zmqSocket[i]->Connect() == 0) { diff --git a/slsDetectorSoftware/src/Module.cpp b/slsDetectorSoftware/src/Module.cpp index 8918eb2e2..7032da6de 100644 --- a/slsDetectorSoftware/src/Module.cpp +++ b/slsDetectorSoftware/src/Module.cpp @@ -458,12 +458,11 @@ std::vector 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]; - memset(mp, 0, sizeof(mp)); - client.Receive(mp, sizeof(mp)); - std::vector retval(mp, mp + nports); + std::vector retval(nports); + client.Receive(retval.data(), + sizeof(decltype(retval[0])) * retval.size()); 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 badchannels; client.Receive(&nch, sizeof(nch)); + std::vector badchannels(nch); if (nch > 0) { - int temp[nch]; - memset(temp, 0, sizeof(temp)); - client.Receive(temp, sizeof(temp)); - badchannels.insert(badchannels.end(), &temp[0], &temp[nch]); - for (int i = 0; i < (int)badchannels.size(); ++i) { + client.Receive(badchannels.data(), + sizeof(badchannels[0]) * badchannels.size()); + for (size_t i = 0; i < badchannels.size(); ++i) { LOG(logDEBUG1) << i << ":" << badchannels[i]; } } // save to file - std::ofstream outfile; - outfile.open(fname.c_str(), std::ios_base::out); + std::ofstream outfile(fname); if (!outfile.is_open()) { throw RuntimeError("Could not create file to save bad channels"); } - for (int i = 0; i < nch; ++i) { - outfile << badchannels[i] << '\n'; - } + for (auto ch : badchannels) + outfile << ch << '\n'; LOG(logDEBUG1) << nch << " bad channels saved to file"; } @@ -1712,10 +1707,9 @@ void Module::setBadChannels(const std::string &fname) { } std::vector badchannels; for (std::string line; std::getline(input_file, line);) { - if (line.find(' ') != std::string::npos) { - line.erase(line.find(' ')); - } - if (line.length() >= 1) { + line.erase(std::remove_if(begin(line), end(line), isspace), + end(line)); // remove space + if (!line.empty()) { std::istringstream iss(line); int ival = 0; iss >> ival; @@ -2139,30 +2133,31 @@ void Module::startPattern() { sendToDetector(F_START_PATTERN); } // Moench std::map 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.Receive(&ret, sizeof(ret)); + client.Send(F_GET_ADDITIONAL_JSON_HEADER); + auto ret = client.Receive(); 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(); + std::string buff(size, '\0'); std::map retval; if (size > 0) { - char retvals[size * 2][SHORT_STR_LENGTH]; - memset(retvals, 0, sizeof(retvals)); - client.Receive(retvals, sizeof(retvals)); - for (int i = 0; i < size; ++i) { - retval[retvals[2 * i]] = retvals[2 * i + 1]; + client.Receive(&buff[0], buff.size()); + std::istringstream iss(buff); + std::string key, value; + while(iss >> key){ + 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(); - int fnum = F_SET_ADDITIONAL_JSON_HEADER; - int ret = FAIL; + std::ostringstream oss; + for (auto& it : jsonHeader) + oss << it.first << ' ' << it.second << ' '; + auto buff = oss.str(); + const auto size = static_cast(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(&size, sizeof(size)); - if (size > 0) { - char args[size * 2][SHORT_STR_LENGTH]; - memset(args, 0, sizeof(args)); - int iarg = 0; - 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)); + client.Send(F_SET_ADDITIONAL_JSON_HEADER); + client.Send(size); + if (size > 0) + client.Send(&buff[0], buff.size()); + + auto ret = client.Receive(); if (ret == FAIL) { char mess[MAX_STR_LENGTH]{}; client.Receive(mess, MAX_STR_LENGTH); diff --git a/slsReceiverSoftware/src/BinaryFile.cpp b/slsReceiverSoftware/src/BinaryFile.cpp index cf6baaf35..48fab967d 100644 --- a/slsReceiverSoftware/src/BinaryFile.cpp +++ b/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), - filefd(nullptr), numFramesInFile(0), numActualPacketsInFile(0), - maxMasterFileSize(2000) { + nunits, nf, dr, portno, smode) { #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)) + diff --git a/slsReceiverSoftware/src/BinaryFile.h b/slsReceiverSoftware/src/BinaryFile.h index 2f725ecd9..9206cedfa 100644 --- a/slsReceiverSoftware/src/BinaryFile.h +++ b/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; - uint64_t numActualPacketsInFile; - const size_t maxMasterFileSize; -}; + uint32_t numFramesInFile = 0; + uint64_t numActualPacketsInFile = 0; + //Make sure this is known at compile time + //TODO! Later away from stack allocation of message + static constexpr size_t maxMasterFileSize = 2000; + +}; \ No newline at end of file diff --git a/slsReceiverSoftware/src/ClientInterface.cpp b/slsReceiverSoftware/src/ClientInterface.cpp index ac45ae0c2..64857950d 100644 --- a/slsReceiverSoftware/src/ClientInterface.cpp +++ b/slsReceiverSoftware/src/ClientInterface.cpp @@ -901,14 +901,12 @@ int ClientInterface::get_frame_index(Interface &socket) { } int ClientInterface::get_missing_packets(Interface &socket) { - std::vector m = impl()->getNumMissingPackets(); - LOG(logDEBUG1) << "missing packets:" << sls::ToString(m); - int retvalsize = m.size(); - uint64_t retval[retvalsize]; - std::copy(std::begin(m), std::end(m), retval); + auto missing_packets = impl()->getNumMissingPackets(); + LOG(logDEBUG1) << "missing packets:" << sls::ToString(missing_packets); + auto size = static_cast(missing_packets.size()); socket.Send(OK); - socket.Send(&retvalsize, sizeof(retvalsize)); - socket.Send(retval, sizeof(retval)); + socket.Send(size); + 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 json; - int size = socket.Receive(); + auto size = socket.Receive(); if (size > 0) { - char args[size * 2][SHORT_STR_LENGTH]; - memset(args, 0, sizeof(args)); - socket.Receive(args, sizeof(args)); - for (int i = 0; i < size; ++i) { - json[args[2 * i]] = args[2 * i + 1]; + std::string buff(size, '\0'); + socket.Receive(&buff[0], buff.size()); + std::istringstream iss(buff); + std::string key, value; + 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 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(buff.size()); socket.sendResult(size); if (size > 0) { - char retvals[size * 2][SHORT_STR_LENGTH]; - 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)); + socket.Send(&buff[0], buff.size()); } return OK; } diff --git a/slsReceiverSoftware/src/DataProcessor.cpp b/slsReceiverSoftware/src/DataProcessor.cpp index e15243136..7a0df2951 100644 --- a/slsReceiverSoftware/src/DataProcessor.cpp +++ b/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]; - memset(result, 0, numResult8Bits * sizeof(uint8_t)); - uint8_t *dest = result; + std::vector result(numResult8Bits); + uint8_t *dest = &result[0]; 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); }