From 86d3fc7e555bf87d2d34352ec269664e5e5bbca5 Mon Sep 17 00:00:00 2001 From: Dhanya Thattil Date: Wed, 29 Jul 2020 16:52:22 +0200 Subject: [PATCH] fixes from review, moving from array to vector to avoid VLA --- .../slsDetectorFunctionList.c | 22 +--- .../src/slsDetectorServer_funcs.c | 23 ++-- slsDetectorSoftware/src/Module.cpp | 104 +++++++++--------- slsDetectorSoftware/src/Module.h | 3 +- 4 files changed, 72 insertions(+), 80 deletions(-) diff --git a/slsDetectorServers/gotthard2DetectorServer/slsDetectorFunctionList.c b/slsDetectorServers/gotthard2DetectorServer/slsDetectorFunctionList.c index b274bf0ba..847793600 100644 --- a/slsDetectorServers/gotthard2DetectorServer/slsDetectorFunctionList.c +++ b/slsDetectorServers/gotthard2DetectorServer/slsDetectorFunctionList.c @@ -374,26 +374,16 @@ void setupDetector() { burstPeriodReg = 0; filter = 0; cdsGain = 0; - for (int i = 0; i < NUM_CLOCKS; ++i) { - clkPhase[i] = 0; - } - for (int i = 0; i < NDAC; ++i) { - dacValues[i] = 0; - } + memset(clkPhase, 0, sizeof(clkPhase)); + memset(dacValues, 0, sizeof(dacValues)); for (int i = 0; i < ONCHIP_NDAC; ++i) { for (int j = 0; j < NCHIP; ++j) { onChipdacValues[i][j] = -1; } } - for (int i = 0; i < NCHIP; ++i) { - for (int j = 0; j < NCHAN; ++j) { - vetoReference[i][j] = 0; - vetoGainIndices[i][j] = 0; - } - for (int j = 0; j < NADC; ++j) { - adcConfiguration[i][j] = 0; - } - } + memset(vetoReference, 0, sizeof(vetoReference)); + memset(vetoGainIndices, 0, sizeof(vetoGainIndices)); + memset(adcConfiguration, 0, sizeof(adcConfiguration)); #ifdef VIRTUAL sharedMemory_setStatus(IDLE); #endif @@ -1789,9 +1779,7 @@ int setVetoReference(int gainIndex, int value) { LOG(logINFO, ("Setting veto reference [chip:-1, G%d, value:0x%x]\n", gainIndex, value)); int values[NCHAN]; - memset(values, 0, sizeof(values)); int gainIndices[NCHAN]; - memset(gainIndices, 0, sizeof(gainIndices)); for (int ich = 0; ich < NCHAN; ++ich) { values[ich] = value; gainIndices[ich] = gainIndex; diff --git a/slsDetectorServers/slsDetectorServer/src/slsDetectorServer_funcs.c b/slsDetectorServers/slsDetectorServer/src/slsDetectorServer_funcs.c index 1225b3192..69e647f2a 100644 --- a/slsDetectorServers/slsDetectorServer/src/slsDetectorServer_funcs.c +++ b/slsDetectorServers/slsDetectorServer/src/slsDetectorServer_funcs.c @@ -6332,16 +6332,14 @@ int set_veto_photon(int file_des) { int args[2] = {-1, -1}; if (receiveData(file_des, args, sizeof(args), INT32) < 0) return printSocketReadError(); - int chipIndex = args[0]; - int numChannels = args[1]; + const int chipIndex = args[0]; + const int numChannels = args[1]; - int gainIndices[args[1]]; - memset(gainIndices, 0, sizeof(gainIndices)); + int gainIndices[numChannels]; if (receiveData(file_des, gainIndices, sizeof(gainIndices), INT32) < 0) return printSocketReadError(); - int values[args[1]]; - memset(values, 0, sizeof(values)); + int values[numChannels]; if (receiveData(file_des, values, sizeof(values), INT32) < 0) return printSocketReadError(); @@ -6353,19 +6351,18 @@ int set_veto_photon(int file_des) { #else // only set if (Server_VerifyLock() == OK) { - - if (chipIndex < -1 || chipIndex >= NCHIP) { - ret = FAIL; - sprintf(mess, "Could not set veto photon. Invalid chip index %d\n", - chipIndex); - LOG(logERROR, (mess)); - } else if (numChannels != NCHAN) { + if (numChannels != NCHAN) { ret = FAIL; sprintf(mess, "Could not set veto photon. Invalid number of channels %d. " "Expected %d\n", numChannels, NCHAN); LOG(logERROR, (mess)); + } else if (chipIndex < -1 || chipIndex >= NCHIP) { + ret = FAIL; + sprintf(mess, "Could not set veto photon. Invalid chip index %d\n", + chipIndex); + LOG(logERROR, (mess)); } else { for (int i = 0; i < NCHAN; ++i) { if (gainIndices[i] < 0 || gainIndices[i] > 2) { diff --git a/slsDetectorSoftware/src/Module.cpp b/slsDetectorSoftware/src/Module.cpp index 4ad8cea7d..c491a49a7 100644 --- a/slsDetectorSoftware/src/Module.cpp +++ b/slsDetectorSoftware/src/Module.cpp @@ -1389,19 +1389,25 @@ void Module::setInjectChannel(const int offsetChannel, sendToDetector(F_SET_INJECT_CHANNEL, args, nullptr); } -void Module::sendVetoPhoton(const int chipIndex, int *gainIndices, - int *values) { - int fnum = F_SET_VETO_PHOTON; - int ret = FAIL; - int nch = shm()->nChan.x; - int args[]{chipIndex, nch}; +void Module::sendVetoPhoton(const int chipIndex, std::vector gainIndices, + std::vector values) { + const int nch = gainIndices.size(); + if (gainIndices.size() != values.size()) { + throw RuntimeError("Number of Gain Indices and values do not match! " + "Gain Indices size: " + + std::to_string(gainIndices.size()) + + ", values size: " + std::to_string(values.size())); + } LOG(logDEBUG1) << "Sending veto photon/file to detector [chip:" << chipIndex << ", nch:" << nch << "]"; + int fnum = F_SET_VETO_PHOTON; + int ret = FAIL; + int args[]{chipIndex, nch}; auto client = DetectorSocket(shm()->hostname, shm()->controlPort); client.Send(&fnum, sizeof(fnum)); client.Send(args, sizeof(args)); - client.Send(gainIndices, sizeof(int) * nch); - client.Send(values, sizeof(int) * nch); + client.Send(gainIndices.data(), sizeof(int) * nch); + client.Send(values.data(), sizeof(int) * nch); client.Receive(&ret, sizeof(ret)); if (ret == FAIL) { char mess[MAX_STR_LENGTH]{}; @@ -1413,6 +1419,7 @@ void Module::sendVetoPhoton(const int chipIndex, int *gainIndices, void Module::getVetoPhoton(const int chipIndex, const std::string &fname) const { + LOG(logDEBUG1) << "Getting veto photon [" << chipIndex << "]\n"; int fnum = F_GET_VETO_PHOTON; int ret = FAIL; auto client = DetectorSocket(shm()->hostname, shm()->controlPort); @@ -1425,16 +1432,18 @@ void Module::getVetoPhoton(const int chipIndex, throw RuntimeError("Detector " + std::to_string(moduleId) + " returned error: " + std::string(mess)); } + int nch = -1; client.Receive(&nch, sizeof(nch)); - LOG(logDEBUG1) << "Getting veto photon [" << chipIndex << "]: " << nch - << " channels\n"; - int gainIndices[nch]; - memset(gainIndices, 0, sizeof(gainIndices)); - client.Receive(gainIndices, sizeof(gainIndices)); - int values[nch]; - memset(values, 0, sizeof(values)); - client.Receive(values, sizeof(values)); + if (nch != shm()->nChan.x) { + throw RuntimeError("Could not get veto photon. Expected " + + std::to_string(shm()->nChan.x) + " channels, got " + + std::to_string(nch)); + } + std::vector gainIndices(nch); + std::vector values(nch); + client.Receive(gainIndices.data(), nch * sizeof(int)); + client.Receive(values.data(), nch * sizeof(int)); // save to file std::ofstream outfile; @@ -1475,12 +1484,8 @@ void Module::setVetoPhoton(const int chipIndex, const int numPhotons, LOG(logDEBUG1) << "Setting veto photon. Reading Gain values from file"; int totalEnergy = numPhotons * energy; - int ch = shm()->nChan.x; - int nRead = 0; - int values[ch]; - int gainIndices[ch]; - memset(values, 0, sizeof(values)); - memset(gainIndices, 0, sizeof(gainIndices)); + std::vector gainIndices; + std::vector values; while (infile.good()) { std::string line; @@ -1503,7 +1508,7 @@ void Module::setVetoPhoton(const int chipIndex, const int numPhotons, if (ss.fail()) { throw RuntimeError("Could not set veto photon. Invalid pedestals, " "gain values or gain thresholds for channel " + - std::to_string(nRead)); + std::to_string(gainIndices.size())); } // caluclate gain index from gain thresholds and threhsold energy @@ -1513,19 +1518,17 @@ void Module::setVetoPhoton(const int chipIndex, const int numPhotons, } else if (totalEnergy < gainThreshold[1]) { gainIndex = 1; } - // calculate ADU values = pedestal + gainvalue * total energy - values[nRead] = - gainPedestal[gainIndex] + (gainValue[gainIndex] * totalEnergy); - gainIndices[nRead] = gainIndex; - ++nRead; - if (nRead >= ch) { - break; - } + gainIndices.push_back(gainIndex); + // calculate ADU value + values.push_back(gainPedestal[gainIndex] + + (gainValue[gainIndex] * totalEnergy)); } - if (nRead != ch) { - throw RuntimeError("Could not set veto photon. Insufficient pedestal " - "for gain values: " + - std::to_string(nRead)); + // check size + if ((int)gainIndices.size() != shm()->nChan.x) { + throw RuntimeError("Could not set veto photon. Invalid number of " + "entries in file. Expected " + + std::to_string(shm()->nChan.x) + ", read " + + std::to_string(gainIndices.size())); } sendVetoPhoton(chipIndex, gainIndices, values); @@ -1558,12 +1561,8 @@ void Module::setVetoFile(const int chipIndex, const std::string &fname) { " for reading"); } - int ch = shm()->nChan.x; - int nRead = 0; - int gainIndices[ch]; - memset(gainIndices, 0, sizeof(gainIndices)); - int values[ch]; - memset(values, 0, sizeof(values)); + std::vector gainIndices; + std::vector values; for (std::string line; std::getline(input_file, line);) { if (line.find('#') != std::string::npos) { @@ -1574,25 +1573,32 @@ void Module::setVetoFile(const int chipIndex, const std::string &fname) { // convert command and string to a vector std::istringstream iss(line); std::string val; - iss >> gainIndices[nRead] >> val; + int gainIndex = -1; + int value = -1; + iss >> gainIndex >> val; if (iss.fail()) { throw RuntimeError("Could not set veto file. Invalid gain " "or reference value for channel " + - std::to_string(nRead)); + std::to_string(gainIndices.size())); } try { - values[nRead] = StringTo(val); + value = StringTo(val); } catch (...) { throw RuntimeError("Could not set veto file. Invalid value " + val + " for channel " + - std::to_string(nRead)); - } - ++nRead; - if (nRead >= ch) { - break; + std::to_string(gainIndices.size())); } + gainIndices.push_back(gainIndex); + values.push_back(value); } } + // check size + if ((int)gainIndices.size() != shm()->nChan.x) { + throw RuntimeError("Could not set veto file. Invalid number of " + "entries in file. Expected " + + std::to_string(shm()->nChan.x) + ", read " + + std::to_string(gainIndices.size())); + } sendVetoPhoton(chipIndex, gainIndices, values); } diff --git a/slsDetectorSoftware/src/Module.h b/slsDetectorSoftware/src/Module.h index 50c0fa275..509af81ea 100644 --- a/slsDetectorSoftware/src/Module.h +++ b/slsDetectorSoftware/src/Module.h @@ -371,7 +371,8 @@ class Module : public virtual slsDetectorDefs { void setBurstPeriod(int64_t value); std::array getInjectChannel() const; void setInjectChannel(const int offsetChannel, const int incrementChannel); - void sendVetoPhoton(const int chipIndex, int *gainIndices, int *values); + void sendVetoPhoton(const int chipIndex, std::vector gainIndices, + std::vector values); void getVetoPhoton(const int chipIndex, const std::string &fname) const; void setVetoPhoton(const int chipIndex, const int numPhotons, const int energy, const std::string &fname);