From 441fab56bf84048ec3b5c1fce1da0f60bfd2683e Mon Sep 17 00:00:00 2001 From: Axel Kohlmeyer Date: Thu, 19 Mar 2026 22:10:14 -0400 Subject: [PATCH 1/5] delete dead code --- src/info.cpp | 1 - 1 file changed, 1 deletion(-) 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); From e6b9d57a15455d89fc3f3884cbdec8d75d34ad12 Mon Sep 17 00:00:00 2001 From: Axel Kohlmeyer Date: Fri, 20 Mar 2026 07:34:04 -0400 Subject: [PATCH 2/5] make certain that read_string() returns a null terminated string --- src/read_restart.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) 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; } From 2ff3d7b6d6f21ab7afb117f6b9e4372580831881 Mon Sep 17 00:00:00 2001 From: Axel Kohlmeyer Date: Fri, 20 Mar 2026 07:35:07 -0400 Subject: [PATCH 3/5] address file pointer issues noted by coverity scan --- src/DPD-REACT/pair_exp6_rx.cpp | 24 ++++++++---------------- src/GRAPHICS/dump_movie.cpp | 11 +---------- src/GRAPHICS/dump_movie.h | 1 - src/ML-POD/fitpod_command.cpp | 23 ++++++----------------- 4 files changed, 15 insertions(+), 44 deletions(-) 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) From a4b22621da1de590647984108c1f2caf54f5057f Mon Sep 17 00:00:00 2001 From: Axel Kohlmeyer Date: Mon, 23 Mar 2026 16:55:34 -0400 Subject: [PATCH 4/5] print warning when auto-deleting file and atomfile style variables --- doc/src/variable.rst | 18 ++++++++++++++++++ doc/utils/sphinx-config/false_positives.txt | 2 ++ src/variable.cpp | 12 ++++++++++-- 3 files changed, 30 insertions(+), 2 deletions(-) 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/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; From 4f1202308169bba48a5f5d194884794e967fb635 Mon Sep 17 00:00:00 2001 From: Axel Kohlmeyer Date: Wed, 25 Mar 2026 11:17:38 -0400 Subject: [PATCH 5/5] fix a couple of typos reported by copilot --- .github/CONTRIBUTING.md | 4 ++-- SECURITY.md | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) 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".