diff --git a/.github/CONTRIBUTING.md b/.github/CONTRIBUTING.md index 46f172202f2..852362d6693 100644 --- a/.github/CONTRIBUTING.md +++ b/.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) diff --git a/SECURITY.md b/SECURITY.md index a5b41952301..2be3f359975 100644 --- a/SECURITY.md +++ b/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". diff --git a/doc/src/variable.rst b/doc/src/variable.rst index 15207340892..7fa8bbfe39b 100644 --- a/doc/src/variable.rst +++ b/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 ` command with the exception of atomfile-style variables. diff --git a/doc/utils/sphinx-config/false_positives.txt b/doc/utils/sphinx-config/false_positives.txt index 41238197591..316cffb533a 100644 --- a/doc/utils/sphinx-config/false_positives.txt +++ b/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 diff --git a/src/DPD-REACT/pair_exp6_rx.cpp b/src/DPD-REACT/pair_exp6_rx.cpp index 2cc7151a19a..78b4a706953 100644 --- a/src/DPD-REACT/pair_exp6_rx.cpp +++ b/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; diff --git a/src/GRAPHICS/dump_movie.cpp b/src/GRAPHICS/dump_movie.cpp index 1a157742173..05475665278 100644 --- a/src/GRAPHICS/dump_movie.cpp +++ b/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 diff --git a/src/GRAPHICS/dump_movie.h b/src/GRAPHICS/dump_movie.h index bebd53e18a2..5147a982c0b 100644 --- a/src/GRAPHICS/dump_movie.h +++ b/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; diff --git a/src/ML-POD/fitpod_command.cpp b/src/ML-POD/fitpod_command.cpp index b5d8b41f46b..bb372093ded 100644 --- a/src/ML-POD/fitpod_command.cpp +++ b/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) diff --git a/src/info.cpp b/src/info.cpp index e5febf093d0..216d19ab714 100644 --- a/src/info.cpp +++ b/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); diff --git a/src/read_restart.cpp b/src/read_restart.cpp index d074765e8b2..bc70710c3a4 100644 --- a/src/read_restart.cpp +++ b/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; } diff --git a/src/variable.cpp b/src/variable.cpp index 44efd52ecd3..55f671931ff 100644 --- a/src/variable.cpp +++ b/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;