diff --git a/src/addRun.cpp b/src/addRun.cpp index 9e361615..5c5ada53 100644 --- a/src/addRun.cpp +++ b/src/addRun.cpp @@ -34,6 +34,7 @@ #include #include #include +#include #include #include @@ -162,24 +163,22 @@ bool addRun_checkFormat(std::string &format) { bool addRun_readInputFiles(const std::string fileName, std::vector &infoVec) { PAddRunInfo info; - char buf[256], str[256]; + char str[256]; std::ifstream fin(fileName.c_str(), std::ifstream::in); std::string line; char *tok{nullptr}; int status, ival; bool lastWasFile{false}; - memset(buf, '\0', sizeof(buf)); memset(str, '\0', sizeof(str)); - while (fin.good()) { - fin.getline(buf, 256); - line = buf; + // read into a std::string: a fixed size line buffer silently drops the rest of + // the input file as soon as one line (e.g. a long data-file path) exceeds it. + while (std::getline(fin, line)) { boost::trim_left(line); if (line.empty()) continue; if (line[0] == '%') continue; - strncpy(buf, line.c_str(), sizeof(buf)); - tok = strtok(buf, " "); + tok = strtok(line.data(), " "); // strtok tokenises in place, line stays the owner if (!strcmp(tok, "file")) { if (lastWasFile) { infoVec.push_back(info); @@ -198,7 +197,7 @@ bool addRun_readInputFiles(const std::string fileName, std::vector lastWasFile = true; } else if (!strcmp(tok, "t0")) { while ((tok = strtok(NULL, ",")) != NULL) { - status = sscanf(tok, "%d%s", &ival, str); + status = sscanf(tok, "%d%255s", &ival, str); if (status != 1) { std::cerr << std::endl; std::cerr << "**ERROR** found t0 argument '" << tok << "' which is not a number." << std::endl; @@ -541,20 +540,19 @@ int main(int argc, char *argv[]) } // read startup file - char startup_path_name[128]; - memset(startup_path_name, '\0', sizeof(startup_path_name)); + std::string startup_path_name{}; std::unique_ptr saxParser = std::make_unique(); std::unique_ptr startupHandler = std::make_unique(); if (!startupHandler->StartupFileFound()) { std::cerr << std::endl << ">> addRun **WARNING** couldn't find " << startupHandler->GetStartupFilePath().Data(); std::cerr << std::endl; } else { - strncpy(startup_path_name, startupHandler->GetStartupFilePath().Data(), sizeof(startup_path_name)); + startup_path_name = startupHandler->GetStartupFilePath().Data(); saxParser->ConnectToHandler("PStartupHandler", startupHandler.get()); //status = saxParser->ParseFile(startup_path_name); // parsing the file as above seems to lead to problems in certain environments; // use the parseXmlFile function instead (see PStartupHandler.cpp for the definition) - status = parseXmlFile(saxParser.get(), startup_path_name); + status = parseXmlFile(saxParser.get(), startup_path_name.c_str()); // check for parse errors if (status) { // error std::cerr << std::endl << ">> addRun **WARNING** Reading/parsing musrfit_startup.xml failed."; diff --git a/src/any2many.cpp b/src/any2many.cpp index 67121939..f3f78dfd 100644 --- a/src/any2many.cpp +++ b/src/any2many.cpp @@ -34,6 +34,7 @@ #include #include #include +#include #include #include @@ -498,20 +499,19 @@ int main(int argc, char *argv[]) } // read startup file - char startup_path_name[128]; - memset(startup_path_name, '\0', sizeof(startup_path_name)); + std::string startup_path_name{}; std::unique_ptr saxParser = std::make_unique(); std::unique_ptr startupHandler = std::make_unique(); if (!startupHandler->StartupFileFound()) { std::cerr << std::endl << ">> any2many **WARNING** couldn't find " << startupHandler->GetStartupFilePath().Data(); std::cerr << std::endl; } else { - strncpy(startup_path_name, startupHandler->GetStartupFilePath().Data(), sizeof(startup_path_name)); + startup_path_name = startupHandler->GetStartupFilePath().Data(); saxParser->ConnectToHandler("PStartupHandler", startupHandler.get()); //status = saxParser->ParseFile(startup_path_name); // parsing the file as above seems to lead to problems in certain environments; // use the parseXmlFile function instead (see PStartupHandler.cpp for the definition) - status = parseXmlFile(saxParser.get(), startup_path_name); + status = parseXmlFile(saxParser.get(), startup_path_name.c_str()); // check for parse errors if (status) { // error std::cerr << std::endl << ">> any2many **WARNING** Reading/parsing musrfit_startup.xml failed."; diff --git a/src/classes/PFourierCanvas.cpp b/src/classes/PFourierCanvas.cpp index 20276d2c..f45c4bf3 100644 --- a/src/classes/PFourierCanvas.cpp +++ b/src/classes/PFourierCanvas.cpp @@ -146,7 +146,13 @@ PFourierCanvas::PFourierCanvas(std::vector &fourier, PIntVector dataS rand.SetSeed(i); style = 20+static_cast(rand.Integer(10)); fMarkerList.push_back(style); - color = TColor::GetColor(static_cast(rand.Integer(255)), static_cast(rand.Integer(255)), static_cast(rand.Integer(255))); + // the rgb values need to be drawn into separate variables first: the evaluation order of + // function arguments is unspecified in C++, hence calling rand.Integer() three times within + // the argument list would yield compiler dependent (r,g,b) permutations. + Int_t r = static_cast(rand.Integer(255)); + Int_t g = static_cast(rand.Integer(255)); + Int_t b = static_cast(rand.Integer(255)); + color = TColor::GetColor(r, g, b); fColorList.push_back(color); } @@ -226,7 +232,13 @@ PFourierCanvas::PFourierCanvas(std::vector &fourier, PIntVector dataS } for (UInt_t i=static_cast(fColorList.size()); i(rand.Integer(255)), static_cast(rand.Integer(255)), static_cast(rand.Integer(255))); + // the rgb values need to be drawn into separate variables first: the evaluation order of + // function arguments is unspecified in C++, hence calling rand.Integer() three times within + // the argument list would yield compiler dependent (r,g,b) permutations. + Int_t r = static_cast(rand.Integer(255)); + Int_t g = static_cast(rand.Integer(255)); + Int_t b = static_cast(rand.Integer(255)); + color = TColor::GetColor(r, g, b); fColorList.push_back(color); } diff --git a/src/classes/PMusrCanvas.cpp b/src/classes/PMusrCanvas.cpp index 58a6ab1d..acf5a357 100644 --- a/src/classes/PMusrCanvas.cpp +++ b/src/classes/PMusrCanvas.cpp @@ -1527,7 +1527,7 @@ void PMusrCanvas::WindowClosed() * \param fileName file name under which the canvas shall be saved. * \param graphicsFormat One of the supported graphics formats. */ -void PMusrCanvas::SaveGraphicsAndQuit(Char_t *fileName, Char_t *graphicsFormat) +void PMusrCanvas::SaveGraphicsAndQuit(const Char_t *fileName, const Char_t *graphicsFormat) { std::cout << std::endl << ">> SaveGraphicsAndQuit: will dump the canvas into a graphics output file (" << graphicsFormat << ") ..."<< std::endl; @@ -2925,7 +2925,13 @@ void PMusrCanvas::HandleDataSet(UInt_t plotNo, UInt_t runNo, PRunData *data) dataHisto->SetLineColor(fColorList[plotNo]); } else { TRandom rand(plotNo); - Int_t color = TColor::GetColor((Int_t)rand.Integer(255), (Int_t)rand.Integer(255), (Int_t)rand.Integer(255)); + // the rgb values need to be drawn into separate variables first: the evaluation order of + // function arguments is unspecified in C++, hence calling rand.Integer() three times within + // the argument list would yield compiler dependent (r,g,b) permutations. + Int_t r = (Int_t)rand.Integer(255); + Int_t g = (Int_t)rand.Integer(255); + Int_t b = (Int_t)rand.Integer(255); + Int_t color = TColor::GetColor(r, g, b); dataHisto->SetMarkerColor(color); dataHisto->SetLineColor(color); } @@ -3058,7 +3064,13 @@ void PMusrCanvas::HandleDataSet(UInt_t plotNo, UInt_t runNo, PRunData *data) theoHisto->SetLineColor(fColorList[plotNo]); } else { TRandom rand(plotNo); - Int_t color = TColor::GetColor((Int_t)rand.Integer(255), (Int_t)rand.Integer(255), (Int_t)rand.Integer(255)); + // the rgb values need to be drawn into separate variables first: the evaluation order of + // function arguments is unspecified in C++, hence calling rand.Integer() three times within + // the argument list would yield compiler dependent (r,g,b) permutations. + Int_t r = (Int_t)rand.Integer(255); + Int_t g = (Int_t)rand.Integer(255); + Int_t b = (Int_t)rand.Integer(255); + Int_t color = TColor::GetColor(r, g, b); theoHisto->SetLineColor(color); } @@ -3108,7 +3120,13 @@ void PMusrCanvas::HandleNonMusrDataSet(UInt_t plotNo, UInt_t runNo, PRunData *da dataHisto->SetLineColor(fColorList[plotNo]); } else { TRandom rand(plotNo); - Int_t color = TColor::GetColor((Int_t)rand.Integer(255), (Int_t)rand.Integer(255), (Int_t)rand.Integer(255)); + // the rgb values need to be drawn into separate variables first: the evaluation order of + // function arguments is unspecified in C++, hence calling rand.Integer() three times within + // the argument list would yield compiler dependent (r,g,b) permutations. + Int_t r = (Int_t)rand.Integer(255); + Int_t g = (Int_t)rand.Integer(255); + Int_t b = (Int_t)rand.Integer(255); + Int_t color = TColor::GetColor(r, g, b); dataHisto->SetMarkerColor(color); dataHisto->SetLineColor(color); } @@ -3138,7 +3156,13 @@ void PMusrCanvas::HandleNonMusrDataSet(UInt_t plotNo, UInt_t runNo, PRunData *da theoHisto->SetLineColor(fColorList[plotNo]); } else { TRandom rand(plotNo); - Int_t color = TColor::GetColor((Int_t)rand.Integer(255), (Int_t)rand.Integer(255), (Int_t)rand.Integer(255)); + // the rgb values need to be drawn into separate variables first: the evaluation order of + // function arguments is unspecified in C++, hence calling rand.Integer() three times within + // the argument list would yield compiler dependent (r,g,b) permutations. + Int_t r = (Int_t)rand.Integer(255); + Int_t g = (Int_t)rand.Integer(255); + Int_t b = (Int_t)rand.Integer(255); + Int_t color = TColor::GetColor(r, g, b); theoHisto->SetLineColor(color); } diff --git a/src/dump_header.cpp b/src/dump_header.cpp index 7561ca75..1897c33d 100644 --- a/src/dump_header.cpp +++ b/src/dump_header.cpp @@ -611,10 +611,10 @@ int dump_header_mud(const std::string fileName, const bool counts) char str[1024]; int success; - char fln[256]; - memset(fln, '\0', sizeof(fln)); - strncpy(fln, fileName.c_str(), sizeof(fln)); - fh = MUD_openRead(fln, &type); + // MUD_openRead() takes a non-const char*, hence the mutable copy. It must not + // be a fixed size buffer: paths longer than its size would silently truncate. + std::string fln(fileName); + fh = MUD_openRead(fln.data(), &type); if (fh == -1) { std::cerr << std::endl << "**ERROR** Couldn't open mud-file " << fileName << ", sorry." << std::endl; return 1; @@ -1019,20 +1019,19 @@ int main(int argc, char *argv[]) // invoke the startup handler in order to get the default search paths to the data files // read startup file - char startup_path_name[128]; - memset(startup_path_name, '\0', sizeof(startup_path_name)); + std::string startup_path_name{}; std::unique_ptr saxParser = std::make_unique(); std::unique_ptr startupHandler = std::make_unique(); if (!startupHandler->StartupFileFound()) { std::cerr << std::endl << ">> musrfit **WARNING** couldn't find " << startupHandler->GetStartupFilePath().Data(); std::cerr << std::endl; } else { - strncpy(startup_path_name, startupHandler->GetStartupFilePath().Data(), sizeof(startup_path_name)); + startup_path_name = startupHandler->GetStartupFilePath().Data(); saxParser->ConnectToHandler("PStartupHandler", startupHandler.get()); //status = saxParser->ParseFile(startup_path_name); // parsing the file as above seems to lead to problems in certain environments; // use the parseXmlFile function instead (see PStartupHandler.cpp for the definition) - int status = parseXmlFile(saxParser.get(), startup_path_name); + int status = parseXmlFile(saxParser.get(), startup_path_name.c_str()); // check for parse errors if (status) { // error std::cerr << std::endl << ">> musrfit **WARNING** Reading/parsing musrfit_startup.xml failed."; diff --git a/src/include/PMusrCanvas.h b/src/include/PMusrCanvas.h index 497d5ea2..78a31db2 100644 --- a/src/include/PMusrCanvas.h +++ b/src/include/PMusrCanvas.h @@ -350,7 +350,7 @@ class PMusrCanvas : public TObject, public TQObject virtual void WindowClosed(); // SLOT /// Saves canvas to graphics file and emits Done signal - virtual void SaveGraphicsAndQuit(Char_t *fileName, Char_t *graphicsFormat); + virtual void SaveGraphicsAndQuit(const Char_t *fileName, const Char_t *graphicsFormat); /// Exports displayed data to ASCII file virtual void ExportData(const Char_t *fileName); diff --git a/src/msr2msr.cpp b/src/msr2msr.cpp index 6852485c..30b8eca9 100644 --- a/src/msr2msr.cpp +++ b/src/msr2msr.cpp @@ -29,6 +29,8 @@ #include #include +#include +#include #include #include @@ -388,7 +390,7 @@ bool msr2msr_theory(char *str, int &tag, int &noOfAddionalParams) * * \param str msr-file line */ -bool msr2msr_is_comment(char *str) +bool msr2msr_is_comment(const char *str) { bool isComment = false; @@ -417,7 +419,7 @@ bool msr2msr_is_comment(char *str) * * \param str msr-file line */ -bool msr2msr_is_whitespace(char *str) +bool msr2msr_is_whitespace(const char *str) { bool isWhitespace = true; @@ -440,29 +442,25 @@ bool msr2msr_is_whitespace(char *str) * \param str msr-file line * \param paramNo parameter number */ -void msr2msr_replace(char *str, int paramNo) +void msr2msr_replace(std::string &str, int paramNo) { - char temp[128]; - char no[16]; + const std::string no{std::to_string(paramNo)}; - memset(temp, 0, sizeof(temp)); + // build the result in a std::string: the previous fixed size scratch buffer + // overflowed for msr-lines longer than its size. + std::string temp; + temp.reserve(str.length()); - snprintf(no, sizeof(no), "%d", paramNo); - - int j=0; - for (unsigned int i=0; i parameter number + temp += no; i += 2; } } - strcpy(str, temp); + str = temp; } //-------------------------------------------------------------------------- @@ -494,25 +492,23 @@ bool msr2msr_finalize_theory(char *fln, int theoryTag, int noOfAddionalParams) return 0; } - char str[256]; + std::string str; int tag = -1; bool success = true; int param = 0; int count = 0; - while (!fin.eof() && success) { - fin.getline(str, sizeof(str)); - - if (strstr(str, "FITPARAMETER")) { + while (std::getline(fin, str) && success) { + if (str.find("FITPARAMETER") != std::string::npos) { tag = MSR_TAG_FITPARAMETER; - } else if (strstr(str, "THEORY")) { + } else if (str.find("THEORY") != std::string::npos) { tag = MSR_TAG_THEORY; } - if ((tag == MSR_TAG_FITPARAMETER) && !strstr(str, "FITPARAMETER")) { + if ((tag == MSR_TAG_FITPARAMETER) && (str.find("FITPARAMETER") == std::string::npos)) { if ((theoryTag == MSR_THEORY_INTERN_FLD) || (theoryTag == MSR_THEORY_INTERN_BESSEL)) { - if (!msr2msr_is_comment(str)) { + if (!msr2msr_is_comment(str.c_str())) { param++; - if (msr2msr_is_whitespace(str)) { + if (msr2msr_is_whitespace(str.c_str())) { // add needed parameters for (int i=0; i " << fln << ": " << ec.message() << std::endl; return false; } - // rm __temp.msr - strcpy(str, "rm __temp.msr"); - if (system(str) == -1) { - std::cerr << "**ERROR** cmd: " << str << " failed." << std::endl; + std::filesystem::remove(std::filesystem::path("__temp.msr"), ec); + if (ec) { + std::cerr << "**ERROR** couldn't remove __temp.msr: " << ec.message() << std::endl; return false; } @@ -677,9 +676,10 @@ int main(int argc, char *argv[]) // check if conversion seems to be OK if (!success) { - snprintf(str, sizeof(str), "rm -rf %s", argv[2]); - if (system(str) == -1) { - std::cerr << "**ERROR** cmd: " << str << " failed." << std::endl; + std::error_code ec; + std::filesystem::remove_all(std::filesystem::path(argv[2]), ec); + if (ec) { + std::cerr << "**ERROR** couldn't remove " << argv[2] << ": " << ec.message() << std::endl; return 0; } } diff --git a/src/musrFT.cpp b/src/musrFT.cpp index ae8f9e5f..6482110a 100644 --- a/src/musrFT.cpp +++ b/src/musrFT.cpp @@ -1020,7 +1020,7 @@ Int_t main(Int_t argc, Char_t *argv[]) } // read startup file - Char_t startup_path_name[128]; + std::string startup_path_name{}; PStartupOptions startup_options; startup_options.writeExpectedChisq = false; startup_options.estimateN0 = true; @@ -1030,12 +1030,12 @@ Int_t main(Int_t argc, Char_t *argv[]) std::cerr << std::endl << ">> musrFT **WARNING** couldn't find " << startupHandler->GetStartupFilePath().Data(); std::cerr << std::endl; } else { - strcpy(startup_path_name, startupHandler->GetStartupFilePath().Data()); + startup_path_name = startupHandler->GetStartupFilePath().Data(); saxParser->ConnectToHandler("PStartupHandler", startupHandler.get()); //status = saxParser->ParseFile(startup_path_name); // parsing the file as above seems to lead to problems in certain environments; // use the parseXmlFile function instead (see PStartupHandler.cpp for the definition) - status = parseXmlFile(saxParser.get(), startup_path_name); + status = parseXmlFile(saxParser.get(), startup_path_name.c_str()); // check for parse errors if (status) { // error std::cerr << std::endl << ">> musrFT **WARNING** Reading/parsing musrfit_startup.xml failed."; diff --git a/src/musredit_qt5/mupp/plotter/PMuppCanvas.cpp b/src/musredit_qt5/mupp/plotter/PMuppCanvas.cpp index 4c685fd7..51121707 100644 --- a/src/musredit_qt5/mupp/plotter/PMuppCanvas.cpp +++ b/src/musredit_qt5/mupp/plotter/PMuppCanvas.cpp @@ -537,7 +537,13 @@ void PMuppCanvas::UpdateGraphs() gg->SetFillColor(kWhite); } else { // random choise of the color TRandom rand(i+j+k); - color = TColor::GetColor((Int_t)rand.Integer(255), (Int_t)rand.Integer(255), (Int_t)rand.Integer(255)); + // the rgb values need to be drawn into separate variables first: the evaluation order + // of function arguments is unspecified in C++, hence calling rand.Integer() three times + // within the argument list would yield compiler dependent (r,g,b) permutations. + Int_t r = (Int_t)rand.Integer(255); + Int_t g = (Int_t)rand.Integer(255); + Int_t b = (Int_t)rand.Integer(255); + color = TColor::GetColor(r, g, b); gg->SetMarkerColor(color); gg->SetLineColor(color); gg->SetFillColor(kWhite); diff --git a/src/musredit_qt6/mupp/plotter/PMuppCanvas.cpp b/src/musredit_qt6/mupp/plotter/PMuppCanvas.cpp index 16aff557..91e06a12 100644 --- a/src/musredit_qt6/mupp/plotter/PMuppCanvas.cpp +++ b/src/musredit_qt6/mupp/plotter/PMuppCanvas.cpp @@ -591,7 +591,13 @@ void PMuppCanvas::UpdateGraphs() gg->SetFillColor(kWhite); } else { // random choise of the color TRandom rand(i+j+k); - color = TColor::GetColor((Int_t)rand.Integer(255), (Int_t)rand.Integer(255), (Int_t)rand.Integer(255)); + // the rgb values need to be drawn into separate variables first: the evaluation order + // of function arguments is unspecified in C++, hence calling rand.Integer() three times + // within the argument list would yield compiler dependent (r,g,b) permutations. + Int_t r = (Int_t)rand.Integer(255); + Int_t g = (Int_t)rand.Integer(255); + Int_t b = (Int_t)rand.Integer(255); + color = TColor::GetColor(r, g, b); gg->SetMarkerColor(color); gg->SetLineColor(color); gg->SetFillColor(kWhite); diff --git a/src/musrfit.cpp b/src/musrfit.cpp index ceaf781a..61185d49 100644 --- a/src/musrfit.cpp +++ b/src/musrfit.cpp @@ -37,7 +37,7 @@ #include #include -//#include +#include #include #include #include @@ -45,6 +45,7 @@ #include #include #include +#include #include #include @@ -188,7 +189,7 @@ void musrfit_write_ascii(TString fln, PRunData *data, int runCounter) * \param fileName file name * \param runList run list collection handler */ -void musrfit_dump_ascii(char *fileName, PRunListCollection *runList) +void musrfit_dump_ascii(const char *fileName, PRunListCollection *runList) { TString fln(fileName); fln.ReplaceAll(".msr", ".dat"); @@ -331,7 +332,7 @@ void musrfit_write_root(TFile &f, TString fln, PRunData *data, int runCounter) * \param fileName file name * \param runList run list connection handler */ -void musrfit_dump_root(char *fileName, PRunListCollection *runList) +void musrfit_dump_root(const char *fileName, PRunListCollection *runList) { TString fln(fileName); fln.ReplaceAll(".msr", ".root"); @@ -508,7 +509,7 @@ int main(int argc, char *argv[]) #endif TString dump(""); - char filename[1024]; + std::string filename{}; // add default shared library path /usr/local/lib if not already persent const char *dsp = gSystem->GetDynamicPath(); @@ -524,11 +525,9 @@ int main(int argc, char *argv[]) } } - memset(filename, '\0', sizeof(filename)); - strcpy(filename, ""); for (int i=1; i> musrfit **ERROR** no msr-file present!" << std::endl; } @@ -643,7 +642,7 @@ int main(int argc, char *argv[]) } // read startup file - char startup_path_name[128]; + std::string startup_path_name{}; std::unique_ptr saxParser = std::make_unique(); std::unique_ptr startupHandler = std::make_unique(reset_startup_file); if (reset_startup_file) // only rewrite musrfit_startup.xml has been requested @@ -652,13 +651,12 @@ int main(int argc, char *argv[]) std::cerr << std::endl << ">> musrfit **WARNING** couldn't find " << startupHandler->GetStartupFilePath().Data(); std::cerr << std::endl; } else { - memset(startup_path_name, '\0', sizeof(startup_path_name)); - strncpy(startup_path_name, startupHandler->GetStartupFilePath().Data(), sizeof(startup_path_name)); + startup_path_name = startupHandler->GetStartupFilePath().Data(); saxParser->ConnectToHandler("PStartupHandler", startupHandler.get()); //status = saxParser->ParseFile(startup_path_name); // parsing the file as above seems to lead to problems in certain environments; // use the parseXmlFile function instead (see PStartupHandler.cpp for the definition) - status = parseXmlFile(saxParser.get(), startup_path_name); + status = parseXmlFile(saxParser.get(), startup_path_name.c_str()); // check for parse errors if (status) { // error std::cerr << std::endl << ">> musrfit **WARNING** Reading/parsing musrfit_startup.xml failed."; @@ -674,9 +672,9 @@ int main(int argc, char *argv[]) // read msr-file std::unique_ptr msrHandler; if (startupHandler) - msrHandler = std::make_unique(filename, &startup_options); + msrHandler = std::make_unique(filename.c_str(), &startup_options); else - msrHandler = std::make_unique(filename); + msrHandler = std::make_unique(filename.c_str()); status = msrHandler->ReadMsrFile(); if (status != PMUSR_SUCCESS) { switch (status) { @@ -796,9 +794,9 @@ int main(int argc, char *argv[]) std::cout << std::endl << "will write dump file ..." << std::endl; dump.ToLower(); if (dump.Contains("ascii")) - musrfit_dump_ascii(filename, runListCollection.get()); + musrfit_dump_ascii(filename.c_str(), runListCollection.get()); else if (dump.Contains("root")) - musrfit_dump_root(filename, runListCollection.get()); + musrfit_dump_root(filename.c_str(), runListCollection.get()); else std::cout << std::endl << "do not know format " << dump.Data() << ", sorry :-| " << std::endl; } @@ -807,16 +805,13 @@ int main(int argc, char *argv[]) if (success) { if (keep_mn2_output && !chisq_only && !fitter->IsScanOnly()) { // 1st rename MINUIT2.OUTPUT - TString fln = TString(filename); - char ext[32]; - strcpy(ext, "-mn2.output"); - fln.ReplaceAll(".msr", 4, ext, strlen(ext)); + TString fln = TString(filename.c_str()); + fln.ReplaceAll(".msr", "-mn2.output"); gSystem->CopyFile("MINUIT2.OUTPUT", fln.Data(), kTRUE); // 2nd rename MINUIT2.ROOT - fln = TString(filename); - strcpy(ext, "-mn2.root"); - fln.ReplaceAll(".msr", 4, ext, strlen(ext)); + fln = TString(filename.c_str()); + fln.ReplaceAll(".msr", "-mn2.root"); gSystem->CopyFile("MINUIT2.root", fln.Data(), kTRUE); } } @@ -826,13 +821,11 @@ int main(int argc, char *argv[]) // swap msr- and mlog-file std::cout << std::endl << ">> swapping msr-, mlog-file ..." << std::endl; // copy msr-file -> __temp.msr - gSystem->CopyFile(filename, "__temp.msr", kTRUE); + gSystem->CopyFile(filename.c_str(), "__temp.msr", kTRUE); // copy mlog-file -> msr-file - TString fln = TString(filename); - char ext[32]; - strcpy(ext, ".mlog"); - fln.ReplaceAll(".msr", 4, ext, strlen(ext)); - gSystem->CopyFile(fln.Data(), filename, kTRUE); + TString fln = TString(filename.c_str()); + fln.ReplaceAll(".msr", ".mlog"); + gSystem->CopyFile(fln.Data(), filename.c_str(), kTRUE); // copy __temp.msr -> mlog-file gSystem->CopyFile("__temp.msr", fln.Data(), kTRUE); // delete __temp.msr diff --git a/src/musrt0.cpp b/src/musrt0.cpp index f75bb55a..0f6a8909 100644 --- a/src/musrt0.cpp +++ b/src/musrt0.cpp @@ -37,6 +37,7 @@ #include #include +#include #include #include @@ -181,7 +182,7 @@ Int_t main(Int_t argc, Char_t *argv[]) Bool_t show_syntax = false; Int_t status; Bool_t success = true; - Char_t filename[1024]; + std::string filename{}; Bool_t getT0FromPromptPeak = false; Bool_t firstGoodBinOffsetPresent = false; Int_t firstGoodBinOffset = 0; @@ -198,7 +199,6 @@ Int_t main(Int_t argc, Char_t *argv[]) if (strstr(dsp, "/usr/local/lib") == nullptr) gSystem->AddDynamicPath("/usr/local/lib"); - memset(filename, '\0', sizeof(filename)); for (int i=1; iGetDynamicPath() << "'" << std::endl << std::endl; return PMUSR_SUCCESS; } else if (strstr(argv[i], ".msr")) { // check for filename - if (strlen(filename) == 0) { - strncpy(filename, argv[i], sizeof(filename)); + if (filename.empty()) { + filename = argv[i]; } else { std::cout << std::endl << "**ERROR** only one file name allowed." << std::endl; show_syntax = true; @@ -262,7 +262,7 @@ Int_t main(Int_t argc, Char_t *argv[]) } } - if (strlen(filename) == 0) { + if (filename.empty()) { std::cout << std::endl << "**ERROR** msr-file missing!" << std::endl; show_syntax = true; } @@ -273,19 +273,19 @@ Int_t main(Int_t argc, Char_t *argv[]) } // read startup file - Char_t startup_path_name[128]; + std::string startup_path_name{}; std::unique_ptr saxParser = std::make_unique(); std::unique_ptr startupHandler = std::make_unique(); if (!startupHandler->StartupFileFound()) { std::cerr << std::endl << ">> musrt0 **WARNING** couldn't find " << startupHandler->GetStartupFilePath().Data(); std::cerr << std::endl; } else { - strcpy(startup_path_name, startupHandler->GetStartupFilePath().Data()); + startup_path_name = startupHandler->GetStartupFilePath().Data(); saxParser->ConnectToHandler("PStartupHandler", startupHandler.get()); //status = saxParser->ParseFile(startup_path_name); // parsing the file as above seems to lead to problems in certain environments; // use the parseXmlFile function instead (see PStartupHandler.cpp for the definition) - status = parseXmlFile(saxParser.get(), startup_path_name); + status = parseXmlFile(saxParser.get(), startup_path_name.c_str()); // check for parse errors if (status) { // error std::cerr << std::endl << ">> musrt0 **WARNING** Reading/parsing musrfit_startup.xml failed."; @@ -294,7 +294,7 @@ Int_t main(Int_t argc, Char_t *argv[]) } // read msr-file - std::unique_ptr msrHandler = std::make_unique(filename); + std::unique_ptr msrHandler = std::make_unique(filename.c_str()); status = msrHandler->ReadMsrFile(); if (status != PMUSR_SUCCESS) { switch (status) { @@ -1165,14 +1165,11 @@ Int_t main(Int_t argc, Char_t *argv[]) // swap msr- and mlog-file // copy msr-file -> __temp.msr - gSystem->CopyFile(filename, "__temp.msr", kTRUE); + gSystem->CopyFile(filename.c_str(), "__temp.msr", kTRUE); // copy mlog-file -> msr-file - TString fln = TString(filename); - Char_t ext[32]; - memset(ext, '\0', sizeof(ext)); - strncpy(ext, ".mlog", sizeof(ext)); - fln.ReplaceAll(".msr", 4, ext, strlen(ext)); - gSystem->CopyFile(fln.Data(), filename, kTRUE); + TString fln = TString(filename.c_str()); + fln.ReplaceAll(".msr", ".mlog"); + gSystem->CopyFile(fln.Data(), filename.c_str(), kTRUE); // copy __temp.msr -> mlog-file gSystem->CopyFile("__temp.msr", fln.Data(), kTRUE); // delete __temp.msr diff --git a/src/musrview.cpp b/src/musrview.cpp index 18a60151..2f629414 100644 --- a/src/musrview.cpp +++ b/src/musrview.cpp @@ -37,6 +37,7 @@ #include #include +#include #include #include @@ -123,20 +124,17 @@ int main(int argc, char *argv[]) bool show_syntax{false}; int status; bool success{true}; - char fileName[128]; + std::string fileName{}; bool fourier{false}; bool avg{false}; bool theoAtData{false}; // theory points only at data points bool graphicsOutput{false}; bool asciiOutput{false}; - char graphicsExtension[128]; + std::string graphicsExtension{}; int timeout{0}; bool show_errMsgBox{false}; std::stringstream errMsg; - memset(fileName, '\0', sizeof(fileName)); - memset(graphicsExtension, '\0', sizeof(graphicsExtension)); - // add default shared library path /usr/local/lib if not already persent const char *dsp = gSystem->GetDynamicPath(); if (strstr(dsp, "/usr/local/lib") == nullptr) @@ -149,8 +147,8 @@ int main(int argc, char *argv[]) } for (int i=1; i saxParser = std::make_unique(); std::unique_ptr startupHandler = std::make_unique(); if (!startupHandler->StartupFileFound()) { std::cerr << std::endl << ">> musrview **WARNING** couldn't find " << startupHandler->GetStartupFilePath().Data(); std::cerr << std::endl; } else { - strncpy(startup_path_name, startupHandler->GetStartupFilePath().Data(), sizeof(startup_path_name)); + startup_path_name = startupHandler->GetStartupFilePath().Data(); saxParser->ConnectToHandler("PStartupHandler", startupHandler.get()); //status = saxParser->ParseFile(startup_path_name); // parsing the file as above seems to lead to problems in certain environments; // use the parseXmlFile function instead (see PStartupHandler.cpp for the definition) - status = parseXmlFile(saxParser.get(), startup_path_name); + status = parseXmlFile(saxParser.get(), startup_path_name.c_str()); // check for parse errors if (status) { // error std::cerr << std::endl << ">> musrview **WARNING** Reading/parsing musrfit_startup.xml failed."; @@ -245,7 +242,7 @@ int main(int argc, char *argv[]) } // read msr-file - std::unique_ptr msrHandler = std::make_unique(fileName); + std::unique_ptr msrHandler = std::make_unique(fileName.c_str()); status = msrHandler->ReadMsrFile(); if (status != PMUSR_SUCCESS) { errMsg << msrHandler->GetLastErrorMsg(); @@ -402,12 +399,12 @@ int main(int argc, char *argv[]) musrCanvas->Connect("Done(Int_t)", "TApplication", &app, "Terminate(Int_t)"); if (graphicsOutput) { - musrCanvas->SaveGraphicsAndQuit(fileName, graphicsExtension); + musrCanvas->SaveGraphicsAndQuit(fileName.c_str(), graphicsExtension.c_str()); } if (asciiOutput) { // generate export data file name - TString str(fileName); + TString str(fileName.c_str()); str.Remove(str.Last('.')); str += ".dat"; // save data in batch mode diff --git a/tests/musrview_check/musrview_check.py b/tests/musrview_check/musrview_check.py index d09c0d3a..bde71bed 100644 --- a/tests/musrview_check/musrview_check.py +++ b/tests/musrview_check/musrview_check.py @@ -21,7 +21,6 @@ import os import shutil import subprocess import sys -import tempfile import time @@ -76,36 +75,56 @@ def main(): msr_basename = os.path.splitext(os.path.basename(args.msr_file))[0] ref_subdir = os.path.join(args.ref_dir, args.test_name) - # ---- run musrview in a temporary directory ------------------------------- + # ---- run musrview -------------------------------------------------------- + # musrview always writes its PNGs next to the msr-file, so the test has to + # run there and clean up after itself. work_dir = os.path.dirname(os.path.abspath(args.msr_file)) - with tempfile.TemporaryDirectory() as tmp_dir: - # snapshot existing PNGs so we only pick up newly created ones - pre_existing = set(glob.glob(os.path.join(work_dir, f"{msr_basename}_*.png"))) + png_glob = os.path.join(work_dir, f"{msr_basename}_*.png") + # Snapshot the PNGs that are already there together with their mtimes. A PNG + # counts as produced by this run if it is either new, or pre-existing but + # rewritten (a leftover from an earlier aborted run -- musrview simply + # overwrites it). Matching on mtime rather than mere existence keeps stale + # leftovers from masking the real output, while still ignoring the PNGs of a + # sibling test running concurrently on the same msr-file. + pre_existing = {p: os.stat(p).st_mtime_ns for p in glob.glob(png_glob)} + + generated = [] + + def collect(): + """Return the PNGs in work_dir that this musrview run created/rewrote.""" + found = [] + for p in glob.glob(png_glob): + if pre_existing.get(p) != os.stat(p).st_mtime_ns: + found.append(p) + return sorted(found) + + def cleanup(): + """Remove everything this run produced -- must happen on every exit path, + otherwise the leftovers pile up in doc/examples.""" + for png in generated: + try: + os.remove(png) + except OSError: + pass + + try: cmd = [args.musrview, args.msr_file, "--png"] + musrview_opts print(f"running: {' '.join(cmd)}") - result = subprocess.run(cmd, capture_output=True, text=True, - cwd=work_dir, - env={**os.environ, "MUSRVIEW_PNG_DIR": tmp_dir}) + result = subprocess.run(cmd, capture_output=True, text=True, cwd=work_dir) if result.returncode != 0: + generated = collect() print(f"**ERROR** musrview returned exit code {result.returncode}") print(result.stdout + result.stderr) return 1 - # collect only newly created PNGs (exclude pre-existing ones). # musrview has already exited by the time subprocess.run() returns, but on # some systems the PNG file's directory entry becomes visible to this # process only after a further delay (observed running under ctest, up to # several seconds) -- poll for a while instead of failing on the first # empty glob. - generated = [] for _ in range(150): - generated = sorted( - p for p in glob.glob(os.path.join(work_dir, f"{msr_basename}_*.png")) - if p not in pre_existing - ) - if not generated: - generated = sorted(glob.glob(os.path.join(tmp_dir, f"{msr_basename}_*.png"))) + generated = collect() if generated: break time.sleep(0.1) @@ -123,10 +142,6 @@ def main(): dst = os.path.join(ref_subdir, os.path.basename(png)) shutil.copy2(png, dst) print(f" saved reference: {dst}") - # clean up generated PNGs from work_dir - for png in generated: - if os.path.dirname(os.path.abspath(png)) == os.path.abspath(work_dir): - os.remove(png) print(f"GENERATE: {len(generated)} reference PNG(s) written to {ref_subdir}") return 0 @@ -153,11 +168,6 @@ def main(): else: print(f"PASS: {name} diff={diff:.6f} <= tol={args.tol:.6f}") - # clean up generated PNGs from work_dir - for png in generated: - if os.path.dirname(os.path.abspath(png)) == os.path.abspath(work_dir): - os.remove(png) - if compared == 0: print("**ERROR** no PNGs were compared") return 1 @@ -168,6 +178,8 @@ def main(): print(f"\nAll {compared} PNG(s) PASSED (tol={args.tol})") return 0 + finally: + cleanup() if __name__ == "__main__": diff --git a/tests/musrview_check/ref/musrview-histo-HAL9500/test-histo-HAL9500_0.png b/tests/musrview_check/ref/musrview-histo-HAL9500/test-histo-HAL9500_0.png index 7b1627be..74d94c8c 100644 Binary files a/tests/musrview_check/ref/musrview-histo-HAL9500/test-histo-HAL9500_0.png and b/tests/musrview_check/ref/musrview-histo-HAL9500/test-histo-HAL9500_0.png differ