fix musrview ctest failures on macOS: RGB order and path buffers

Three unrelated issues made the musrview_check tests fail on macOS while
passing on Linux.

1. Unspecified argument evaluation order

   TColor::GetColor(rand.Integer(255), rand.Integer(255), rand.Integer(255))

   evaluates its (side-effecting) arguments in an unspecified order: clang
   goes left-to-right, gcc right-to-left, so the same seed yielded RGB on
   one platform and BGR on the other. Visible whenever more runs are
   plotted than the startup xml colour list provides, e.g.
   test-histo-HAL9500.msr with 16 runs against 10 colours.

   The rgb values are now drawn into separate variables first. This keeps
   the clang result and changes gcc to match, hence the regenerated
   reference PNG for musrview-histo-HAL9500 (99.94% of the differing
   pixels were exact R<->B swaps).

   Fixed in PMusrCanvas, PFourierCanvas and both mupp PMuppCanvas copies.

2. Fixed size path buffers

   char fileName[128] plus strncpy(dst, src, sizeof(dst)) truncates *and*
   leaves the buffer unterminated once the path reaches the buffer size.
   musrview then failed on a 132 character msr-path in doc/examples/ViewOpts.
   musrFT and musrt0 were worse: an unbounded strcpy of the startup file
   path into char startup_path_name[128], i.e. a stack buffer overflow.

   All path/filename buffers in the drivers are now std::string:
   musrview, musrFT, musrt0, musrfit, any2many, addRun, dump_header.
   PMusrCanvas::SaveGraphicsAndQuit() takes const Char_t* accordingly.

   Along the way in the same files:
   - msr2msr_replace() wrote a 256 byte line into char temp[128]
   - msr2msr assembled "cp"/"rm" shell commands from paths in a 256 byte
     buffer; replaced by std::filesystem
   - msr2msr and addRun read lines with getline(buf, N), which silently
     abandons the rest of the file on the first over-long line
   - addRun: bound the unbounded sscanf "%s" to "%255s"
   - dropped scratch buffers that only held a string literal, in favour of
     TString::ReplaceAll(const char*, const char*)

3. musrview_check.py left its PNGs behind on failure

   The two early error returns skipped the cleanup, and since generated
   PNGs were identified by "not in the pre-run snapshot", one leftover file
   permanently masked the real output of that test: musrview overwrites the
   stale PNG, so it was there, just filtered out. One failure thus poisoned
   every later run (30 of the 37 observed failures) after a 15 s poll each.

   Cleanup now runs in a finally block on every exit path, and generated
   PNGs are detected by mtime instead, which sees rewritten leftovers while
   staying safe for a sibling test running concurrently under ctest -j.
   Also dropped the MUSRVIEW_PNG_DIR env var and its tmp-dir fallback: it
   is read nowhere in src/, so the fallback was dead code that guaranteed
   the full 15 s poll before every "no PNGs found" failure.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

src/addRun.cpp

@@ -34,6 +34,7 @@
 #include <cstdio>
 #include <cstdlib>
 #include <cstring>
+#include <string>
 
 #include <iostream>
 #include <fstream>
@@ -162,24 +163,22 @@ bool addRun_checkFormat(std::string &format) {
 bool addRun_readInputFiles(const std::string fileName, std::vector<PAddRunInfo> &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<PAddRunInfo>
       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<TSAXParser> saxParser = std::make_unique<TSAXParser>();
   std::unique_ptr<PStartupHandler> startupHandler = std::make_unique<PStartupHandler>();
   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.";

src/any2many.cpp

@@ -34,6 +34,7 @@
 #include <cstdio>
 #include <cstdlib>
 #include <cstring>
+#include <string>
 
 #include <iostream>
 #include <fstream>
@@ -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<TSAXParser> saxParser = std::make_unique<TSAXParser>();
   std::unique_ptr<PStartupHandler> startupHandler = std::make_unique<PStartupHandler>();
   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.";

src/classes/PFourierCanvas.cpp

@@ -146,7 +146,13 @@ PFourierCanvas::PFourierCanvas(std::vector<PFourier*> &fourier, PIntVector dataS
     rand.SetSeed(i);
     style = 20+static_cast<Int_t>(rand.Integer(10));
     fMarkerList.push_back(style);
-    color = TColor::GetColor(static_cast<Int_t>(rand.Integer(255)), static_cast<Int_t>(rand.Integer(255)), static_cast<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 = static_cast<Int_t>(rand.Integer(255));
+    Int_t g = static_cast<Int_t>(rand.Integer(255));
+    Int_t b = static_cast<Int_t>(rand.Integer(255));
+    color = TColor::GetColor(r, g, b);
     fColorList.push_back(color);
   }
 
@@ -226,7 +232,13 @@ PFourierCanvas::PFourierCanvas(std::vector<PFourier*> &fourier, PIntVector dataS
   }
   for (UInt_t i=static_cast<UInt_t>(fColorList.size()); i<fourier.size(); i++) {
     rand.SetSeed(i);
-    color = TColor::GetColor(static_cast<Int_t>(rand.Integer(255)), static_cast<Int_t>(rand.Integer(255)), static_cast<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 = static_cast<Int_t>(rand.Integer(255));
+    Int_t g = static_cast<Int_t>(rand.Integer(255));
+    Int_t b = static_cast<Int_t>(rand.Integer(255));
+    color = TColor::GetColor(r, g, b);
     fColorList.push_back(color);
   }
 

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);
   }
 

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<TSAXParser> saxParser = std::make_unique<TSAXParser>();
   std::unique_ptr<PStartupHandler> startupHandler = std::make_unique<PStartupHandler>();
   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.";

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);
 

src/msr2msr.cpp

@@ -29,6 +29,8 @@
 
 #include <iostream>
 #include <fstream>
+#include <filesystem>
+#include <string>
 
 #include <cctype>
 #include <cstring>
@@ -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<strlen(str); i++) {
+  for (std::string::size_type i=0; i<str.length(); i++) {
     if (str[i] != '_') {
-      temp[j] = str[i];
-      j++;
-    } else {
-      for (unsigned int k=0; k<strlen(no); k++)
-        temp[j+k] = no[k];
-      j += strlen(no);
+      temp += str[i];
+    } else { // '_x_' placeholder -> 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<noOfAddionalParams; i++) {
               fout << "        " << param+i << " frac" << i+1 << "       0.333333 0.0        none" << std::endl;
@@ -524,7 +520,7 @@ bool msr2msr_finalize_theory(char *fln, int theoryTag, int noOfAddionalParams)
 
     if (tag == MSR_TAG_THEORY) {
       if ((theoryTag == MSR_THEORY_INTERN_FLD) || (theoryTag == MSR_THEORY_INTERN_BESSEL)) {
-        if (strstr(str, "_x_")) {
+        if (str.find("_x_") != std::string::npos) {
           msr2msr_replace(str, param+count);
           count++;
         }
@@ -539,16 +535,19 @@ bool msr2msr_finalize_theory(char *fln, int theoryTag, int noOfAddionalParams)
   fin.close();
 
 
-  // cp __temp.msr fln
-  snprintf(str, sizeof(str), "cp __temp.msr %s", fln);
-  if (system(str) == -1) {
-    std::cerr << "**ERROR** cmd: " << str << " failed." << std::endl;
+  // cp __temp.msr fln, and rm __temp.msr afterwards. Done via std::filesystem
+  // rather than system(): a shell command assembled in a fixed size buffer would
+  // silently truncate for long paths (and choke on paths containing blanks).
+  std::error_code ec;
+  std::filesystem::copy_file(std::filesystem::path("__temp.msr"), std::filesystem::path(fln),
+                             std::filesystem::copy_options::overwrite_existing, ec);
+  if (ec) {
+    std::cerr << "**ERROR** couldn't copy __temp.msr -> " << 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;
     }
   }

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.";

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);

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);

src/musrfit.cpp

@@ -37,7 +37,7 @@
 
 #include <cstdio>
 #include <cstdlib>
-//#include <string.h>
+#include <cstring>
 #include <sys/types.h>
 #include <unistd.h>
 #include <signal.h>
@@ -45,6 +45,7 @@
 #include <iostream>
 #include <fstream>
 #include <memory>
+#include <string>
 
 #include <TSAXParser.h>
 #include <TString.h>
@@ -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<argc; i++) {
     if (strstr(argv[i], ".msr")) {
-      strncpy(filename, argv[i], sizeof(filename));
+      filename = argv[i];
     } else if (!strcmp(argv[i], "-k") || !strcmp(argv[i], "--keep-mn2-output")) {
       keep_mn2_output = true;
     } else if (!strcmp(argv[i], "-c") || !strcmp(argv[i], "--chisq-only")) {
@@ -622,7 +621,7 @@ int main(int argc, char *argv[])
   }
 
   // check if a filename is present
-  if ((strlen(filename) == 0) && !reset_startup_file) {
+  if (filename.empty() && !reset_startup_file) {
     show_syntax = true;
     std::cout << std::endl << ">> 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<TSAXParser> saxParser = std::make_unique<TSAXParser>();
   std::unique_ptr<PStartupHandler> startupHandler = std::make_unique<PStartupHandler>(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<PMsrHandler> msrHandler;
   if (startupHandler)
-    msrHandler = std::make_unique<PMsrHandler>(filename, &startup_options);
+    msrHandler = std::make_unique<PMsrHandler>(filename.c_str(), &startup_options);
   else
-    msrHandler = std::make_unique<PMsrHandler>(filename);
+    msrHandler = std::make_unique<PMsrHandler>(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

src/musrt0.cpp

@@ -37,6 +37,7 @@
 
 #include <iostream>
 #include <memory>
+#include <string>
 
 #include <TApplication.h>
 #include <TSAXParser.h>
@@ -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; i<argc; i++) {
     if (!strcmp(argv[i], "--version")) {
 #ifdef HAVE_CONFIG_H
@@ -223,8 +223,8 @@ Int_t main(Int_t argc, Char_t *argv[])
       std::cout << std::endl << "  '" << gSystem->GetDynamicPath() << "'" << 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<TSAXParser> saxParser = std::make_unique<TSAXParser>();
   std::unique_ptr<PStartupHandler> startupHandler = std::make_unique<PStartupHandler>();
   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<PMsrHandler> msrHandler = std::make_unique<PMsrHandler>(filename);
+  std::unique_ptr<PMsrHandler> msrHandler = std::make_unique<PMsrHandler>(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

src/musrview.cpp

@@ -37,6 +37,7 @@
 
 #include <iostream>
 #include <memory>
+#include <string>
 #include <vector>
 #include <sstream>
 
@@ -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<argc; i++) {
     if (strstr(argv[i], ".msr") || strstr(argv[i], ".mlog")) {
-      if (strlen(fileName) == 0) {
-        strncpy(fileName, argv[i], sizeof(fileName));
+      if (fileName.empty()) {
+        fileName = argv[i];
       } else {
         std::cerr << std::endl << "**ERROR** only one file name allowed." << std::endl;
         show_syntax = true;
@@ -189,7 +187,7 @@ int main(int argc, char *argv[])
                !strcmp(argv[i], "--xpm") || !strcmp(argv[i], "--root")) {
 
       graphicsOutput = true;
-      strncpy(graphicsExtension, argv[i]+2, sizeof(graphicsExtension));
+      graphicsExtension = argv[i]+2;
     } else if (!strcmp(argv[i], "--ascii")) {
       asciiOutput = true;
     } else if (!strcmp(argv[i], "--timeout")) {
@@ -220,20 +218,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<TSAXParser> saxParser = std::make_unique<TSAXParser>();
   std::unique_ptr<PStartupHandler> startupHandler = std::make_unique<PStartupHandler>();
   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<PMsrHandler> msrHandler = std::make_unique<PMsrHandler>(fileName);
+  std::unique_ptr<PMsrHandler> msrHandler = std::make_unique<PMsrHandler>(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

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__":