[pull] develop from lammps:develop

.github/CONTRIBUTING.md

@@ -2,7 +2,7 @@
 
 Thank you for considering to contribute to the LAMMPS software project.
 
-The following is a set of guidelines as well as explanations of policies and work flows for contributing to the LAMMPS molecular dynamics software project. These guidelines focus on submitting issues or pull requests on the LAMMPS GitHub project.
+The following is a set of guidelines as well as explanations of policies and workflows for contributing to the LAMMPS molecular dynamics software project. These guidelines focus on submitting issues or pull requests on the LAMMPS GitHub project.
 
 Thus please also have a look at:
 * [The guide for submitting new features in the LAMMPS manual](https://docs.lammps.org/Modify_contribute.html)
@@ -19,7 +19,7 @@ Thus please also have a look at:
 * [Suggesting Enhancements](#suggesting-enhancements)
 * [Contributing Code](#contributing-code)
 
-[GitHub Work flows](#github-workflows)
+[GitHub Workflows](#github-workflows)
 * [Issues](#issues)
 * [Pull Requests](#pull-requests)
 

SECURITY.md

@@ -13,7 +13,7 @@ libraries with experimental research software, that are not validated
 and tested by the LAMMPS developers, so it is easy to import bad
 behavior from calling functions in one of those libraries.
 
-Thus is is quite easy to crash LAMMPS through malicious input and do all
+Thus it is quite easy to crash LAMMPS through malicious input and do all
 kinds of file system manipulations.  And because of that LAMMPS should
 **NEVER** be compiled or **run** as superuser, either from a "root" or
 "administrator" account directly or indirectly via "sudo" or "su".

doc/src/variable.rst

@@ -227,6 +227,24 @@ from the list of active variables, and is thus available to be
 re-defined in a subsequent variable command.  The *delete* style does
 the same thing.
 
+.. versionchanged:: TBD
+
+.. admonition:: Auto-deleted variables can lead to problems
+   :class: warning
+
+   Special care must be taken when iterated variables, e.g. file style
+   variables are exhausted and thus get deleted **during** a run.  For
+   performance reasons, many features in LAMMPS cache how variables are
+   looked up during a run or minimization for the duration of that run
+   or minimization and this can lead to unexpected behavior when
+   variables get auto-deleted.  The cached index can be out of range or
+   point to the wrong variable.  Thus, LAMMPS will print a warning when
+   a file or atomfile is exhausted and auto-deleted.  This can be
+   avoided by making certain that those variables have additional
+   elements.  This condition is rare, but hard to debug, so make certain
+   that when you see this warning, that this is the intended behavior
+   and be warned of the potential side effects.
+
 Variables are **not** deleted by the :doc:`clear <clear>` command with
 the exception of atomfile-style variables.
 

doc/utils/sphinx-config/false_positives.txt

@@ -524,6 +524,7 @@ checkbox
 checkmark
 checkqeq
 checksum
+checksums
 chemflag
 chemistries
 Chemnitz
@@ -3006,6 +3007,7 @@ pc
 pcg
 pchain
 Pchain
+pclose
 pcmoves
 Pdamp
 pdb

src/DPD-REACT/pair_exp6_rx.cpp

@@ -759,10 +759,8 @@ void PairExp6rx::read_file(char *file)
       n = strlen(line);
       if (comm->me == 0) {
         ptr = fgets(&line[n],MAXLINE-n,fp);
-        if (ptr == nullptr) {
-          eof = 1;
-          fclose(fp);
-        } else n = strlen(line) + 1;
+        if (ptr == nullptr) eof = 1;
+        else n = strlen(line) + 1;
       }
       MPI_Bcast(&eof,1,MPI_INT,0,world);
       if (eof) break;
@@ -827,13 +825,11 @@ void PairExp6rx::read_file2(char *file)
 
   // open file on proc 0
 
-  FILE *fp;
-  fp = nullptr;
+  SafeFilePtr fp;
   if (comm->me == 0) {
     fp = fopen(file,"r");
     if (fp == nullptr)
-      error->one(FLERR,"Cannot open polynomial file {}: {}",
-                                   file,utils::getsyserror());
+      error->one(FLERR,"Cannot open polynomial file {}: {}", file, utils::getsyserror());
   }
 
   // one set of params can span multiple lines
@@ -845,10 +841,8 @@ void PairExp6rx::read_file2(char *file)
   while (true) {
     if (comm->me == 0) {
       ptr = fgets(line,MAXLINE,fp);
-      if (ptr == nullptr) {
-        eof = 1;
-        fclose(fp);
-      } else n = strlen(line) + 1;
+      if (ptr == nullptr) eof = 1;
+      else n = strlen(line) + 1;
     }
     MPI_Bcast(&eof,1,MPI_INT,0,world);
     if (eof) break;
@@ -867,10 +861,8 @@ void PairExp6rx::read_file2(char *file)
       n = strlen(line);
       if (comm->me == 0) {
         ptr = fgets(&line[n],MAXLINE-n,fp);
-        if (ptr == nullptr) {
-          eof = 1;
-          fclose(fp);
-        } else n = strlen(line) + 1;
+        if (ptr == nullptr) eof = 1;
+        else n = strlen(line) + 1;
       }
       MPI_Bcast(&eof,1,MPI_INT,0,world);
       if (eof) break;

src/GRAPHICS/dump_movie.cpp

@@ -33,15 +33,6 @@ DumpMovie::DumpMovie(LAMMPS *lmp, int narg, char **arg) : DumpImage(lmp, narg, a
   filetype = PPM;
   bitrate = 2000;
   framerate = 24;
-  fp = nullptr;
-}
-
-/* ---------------------------------------------------------------------- */
-
-DumpMovie::~DumpMovie()
-{
-  if (fp) platform::pclose(fp);
-  fp = nullptr;
 }
 
 /* ---------------------------------------------------------------------- */
@@ -54,9 +45,9 @@ void DumpMovie::openfile()
     auto moviecmd = fmt::format("ffmpeg -v error -y -r {:.2f} -f image2pipe -c:v ppm -i - "
                                 "-r 24.0 -b:v {}k {}",
                                 framerate, bitrate, filename);
+    fp.set_pclose();
     fp = platform::popen(moviecmd, "w");
 #else
-    fp = nullptr;
     error->one(FLERR, "Support for writing movies not included");
 #endif
 

src/GRAPHICS/dump_movie.h

@@ -27,7 +27,6 @@ namespace LAMMPS_NS {
 class DumpMovie : public DumpImage {
  public:
   DumpMovie(LAMMPS *, int, char **);
-  ~DumpMovie() override;
 
   void openfile() override;
   void init_style() override;

src/ML-POD/fitpod_command.cpp

@@ -154,7 +154,7 @@ void FitPOD::command(int narg, char **arg)
 
       if (comm->me == 0) {    // save coefficients into a text file
         std::string filename = traindata.filenametag + "_coefficients" + ".pod";
-        FILE *fp = fopen(filename.c_str(), "w");
+        SafeFilePtr fp = fopen(filename.c_str(), "w");
 
         int nCoeffAll = desc.nCoeffAll;
         int n1 = 0, n2 = 0;
@@ -173,7 +173,6 @@ void FitPOD::command(int narg, char **arg)
         for (int count = 0; count < n2; count++) {
           utils::print(fp, "{:<10.{}f}\n", fastpodptr->Centroids[count], 14);
         }
-        fclose(fp);
       }
     }
 
@@ -369,10 +368,7 @@ int FitPOD::read_data_file(double *fitting_weights, std::string &file_format,
       // Get next line.
       if (comm->me == 0) {
         ptr = fgets(line, MAXLINE, fpdata);
-        if (ptr == nullptr) {
-          eof = 1;
-          fclose(fpdata);
-        }
+        if (ptr == nullptr) eof = 1;
       }
       MPI_Bcast(&eof, 1, MPI_INT, 0, world);
       if (eof) break;
@@ -395,10 +391,7 @@ int FitPOD::read_data_file(double *fitting_weights, std::string &file_format,
         // Get next line.
         if (comm->me == 0) {
           ptr = fgets(line, MAXLINE, fpdata);
-          if (ptr == nullptr) {
-            eof = 1;
-            fclose(fpdata);
-          }
+          if (ptr == nullptr) eof = 1;
         }
         MPI_Bcast(&eof, 1, MPI_INT, 0, world);
         if (eof) break;
@@ -1734,10 +1727,9 @@ void FitPOD::print_analysis(const datastruct &data, double *outarray, double *er
   std::string filename_analysis =
       fmt::format("{}_{}_analysis.pod", data.filenametag, data.training ? "training" : "test");
 
-  FILE *fp_errors = nullptr;
-  FILE *fp_analysis = nullptr;
-  fp_errors = fopen(filename_errors.c_str(), "w");
-  fp_analysis = fopen(filename_analysis.c_str(), "w");
+  SafeFilePtr fp_errors(fopen(filename_errors.c_str(), "w"));
+  SafeFilePtr fp_analysis(fopen(filename_analysis.c_str(), "w"));
+  if (!fp_errors || !fp_analysis) return;
 
   std::string mystr =
       fmt::format("**************** Begin of Error Analysis for the {} Data Set ****************\n",
@@ -1802,9 +1794,6 @@ void FitPOD::print_analysis(const datastruct &data, double *outarray, double *er
 
   utils::logmesg(lmp, mystr);
   utils::print(fp_errors, mystr);
-
-  fclose(fp_errors);
-  fclose(fp_analysis);
 }
 
 void FitPOD::error_analysis(const datastruct &data, double *coeff)

src/info.cpp

@@ -490,7 +490,6 @@ void Info::command(int narg, char **arg)
   }
 
   if (flags & GROUPS) {
-    int ngroup = group->ngroup;
     char **names = group->names;
     int *dynamic = group->dynamic;
     fputs("\nGroup information:\n",out);

src/read_restart.cpp

@@ -1086,9 +1086,10 @@ char *ReadRestart::read_string()
 {
   int n = read_int();
   if (n < 0) error->all(FLERR,"Illegal size string or corrupt restart");
-  auto *value = new char[n];
+  auto *value = new char[n+1];
   if (me == 0) utils::sfread(FLERR,value,sizeof(char),n,fp,nullptr,error);
-  MPI_Bcast(value,n,MPI_CHAR,0,world);
+  value[n] = '\0';
+  MPI_Bcast(value,n+1,MPI_CHAR,0,world);
   return value;
 }
 

src/variable.cpp

@@ -798,6 +798,7 @@ int Variable::next(int narg, char **arg)
       int done = reader[ivar]->read_scalar(data[ivar][0]);
       if (done) {
         flag = 1;
+        if (comm->me == 0) error->warning(FLERR, "Auto-deleting variable {}\n", arg[iarg]);
         remove(ivar);
       }
     }
@@ -809,6 +810,7 @@ int Variable::next(int narg, char **arg)
       int done = reader[ivar]->read_peratom();
       if (done) {
         flag = 1;
+        if (comm->me == 0) error->warning(FLERR, "Auto-deleting variable {}\n", arg[iarg]);
         remove(ivar);
       }
     }
@@ -4908,7 +4910,10 @@ int Variable::special_function(const std::string &word, char *contents, Tree **t
     if (style[ivar] == SCALARFILE) {
       double value = std::stod(data[ivar][0]);
       int done = reader[ivar]->read_scalar(data[ivar][0]);
-      if (done) remove(ivar);
+      if (done) {
+        if (comm->me == 0) error->warning(FLERR, "Auto-deleting variable {}\n", args[0]);
+        remove(ivar);
+      }
 
       if (tree) {
         auto *newtree = new Tree();
@@ -4930,7 +4935,10 @@ int Variable::special_function(const std::string &word, char *contents, Tree **t
       memcpy(result,reader[ivar]->fixstore->vstore,(atom->nlocal*sizeof(double))&MEMCPYMASK);
 
       int done = reader[ivar]->read_peratom();
-      if (done) remove(ivar);
+      if (done) {
+        if (comm->me == 0) error->warning(FLERR, "Auto-deleting variable {}\n", args[0]);
+        remove(ivar);
+      }
 
       auto *newtree = new Tree();
       newtree->type = ATOMARRAY;