From c8fd217e95ca78b07f8ca33fc496e1f03094015c Mon Sep 17 00:00:00 2001 From: Mark Olesen Date: Fri, 22 Jun 2018 15:18:41 +0200 Subject: [PATCH] BUG: Foam::system command swallows first argument (fixes #901) --- .../viewFactorsGen/viewFactorsGen.C | 2 +- src/OSspecific/POSIX/POSIX.C | 79 +++---------------- .../dynamicLibrary/dynamicCode/dynamicCode.C | 2 +- .../primitives/strings/lists/CStringList.H | 4 + .../primitives/strings/lists/CStringListI.H | 6 ++ .../solarLoad/faceShading/faceShading.C | 2 +- 6 files changed, 22 insertions(+), 73 deletions(-) diff --git a/applications/utilities/preProcessing/viewFactorsGen/viewFactorsGen.C b/applications/utilities/preProcessing/viewFactorsGen/viewFactorsGen.C index 82ae5dc488..30ec3a8c8d 100644 --- a/applications/utilities/preProcessing/viewFactorsGen/viewFactorsGen.C +++ b/applications/utilities/preProcessing/viewFactorsGen/viewFactorsGen.C @@ -193,7 +193,7 @@ void writeRays Pout<< "cmd: objToVTK " << fName.c_str() << endl; - stringList cmd{"objToVTK", fName, fName.lessExt().ext("vtk")}; + stringList cmd({"objToVTK", fName, fName.lessExt().ext("vtk")}); Foam::system(cmd); } diff --git a/src/OSspecific/POSIX/POSIX.C b/src/OSspecific/POSIX/POSIX.C index 8c0072ec7d..c042b9da05 100644 --- a/src/OSspecific/POSIX/POSIX.C +++ b/src/OSspecific/POSIX/POSIX.C @@ -1408,7 +1408,7 @@ int Foam::system(const std::string& command, const bool bg) reinterpret_cast(0) ); - // obviously failed, since exec should not return at all + // Obviously failed, since exec should not return FatalErrorInFunction << "exec failed: " << command << exit(FatalError); @@ -1426,9 +1426,7 @@ int Foam::system(const std::string& command, const bool bg) int Foam::system(const CStringList& command, const bool bg) { - const int argc = command.size(); - - if (!argc) + if (command.empty()) { // Treat an empty command as a successful no-op. // For consistency with POSIX (man sh) behaviour for (sh -c command), @@ -1459,14 +1457,10 @@ int Foam::system(const CStringList& command, const bool bg) // Close or redirect file descriptors redirects(); + // execvp searches the path, uses the current environ + (void) ::execvp(command[0], command.strings()); - // Need command and arguments separately. - // args is a nullptr-terminated list of c-strings - - // execvp uses the current environ - (void) ::execvp(command[0], command.strings(1)); - - // obviously failed, since exec should not return at all + // Obviously failed, since exec should not return FatalErrorInFunction << "exec(" << command[0] << ", ...) failed" << exit(FatalError); @@ -1484,70 +1478,15 @@ int Foam::system(const CStringList& command, const bool bg) int Foam::system(const Foam::UList& command, const bool bg) { - // In the future simply call the CStringList version: - // - // const CStringList cmd(command); - // return Foam::system(cmd, bg); - - const int argc = command.size(); - - if (!argc) + if (command.empty()) { // Treat an empty command as a successful no-op. - // For consistency with POSIX (man sh) behaviour for (sh -c command), - // which is what is mostly being replicated here. return 0; } - // NB: use vfork, not fork! - // vfork behaves more like a thread and avoids copy-on-write problems - // triggered by fork. - // The normal system() command has a fork buried in it that causes - // issues with infiniband and openmpi etc. - - const pid_t child_pid = ::vfork(); - - if (child_pid == -1) - { - FatalErrorInFunction - << "vfork() failed for system command " << command[0] - << exit(FatalError); - - return -1; // fallback error value - } - else if (child_pid == 0) - { - // In child - - // Close or redirect file descriptors - redirects(); - - - // Need command and arguments separately. - // args is a nullptr-terminated list of c-strings - - CStringList args(SubList(command, 0)); - if (argc > 1) - { - args.reset(SubList(command, argc-1, 1)); - } - - // execvp uses the current environ - (void) ::execvp(command[0].c_str(), args.strings()); - - // obviously failed, since exec should not return at all - FatalErrorInFunction - << "exec(" << command[0] << ", ...) failed" - << exit(FatalError); - - return -1; // fallback error value - } - - - // In parent - // - started as background process, or blocking wait for the child - - return (bg ? 0 : waitpid(child_pid)); + // Make a deep copy as C-strings + const CStringList cmd(command); + return Foam::system(cmd, bg); } diff --git a/src/OpenFOAM/db/dynamicLibrary/dynamicCode/dynamicCode.C b/src/OpenFOAM/db/dynamicLibrary/dynamicCode/dynamicCode.C index 2b5b5e2491..761391b605 100644 --- a/src/OpenFOAM/db/dynamicLibrary/dynamicCode/dynamicCode.C +++ b/src/OpenFOAM/db/dynamicLibrary/dynamicCode/dynamicCode.C @@ -498,7 +498,7 @@ bool Foam::dynamicCode::copyOrCreateFiles(const bool verbose) const bool Foam::dynamicCode::wmakeLibso() const { - stringList cmd{"wmake", "-s", "libso", this->codePath()}; + stringList cmd({"wmake", "-s", "libso", this->codePath()}); // NOTE: could also resolve wmake command explicitly // cmd[0] = stringOps::expand("$WM_PROJECT_DIR/wmake/wmake"); diff --git a/src/OpenFOAM/primitives/strings/lists/CStringList.H b/src/OpenFOAM/primitives/strings/lists/CStringList.H index 55c48c3f9c..649293c7a0 100644 --- a/src/OpenFOAM/primitives/strings/lists/CStringList.H +++ b/src/OpenFOAM/primitives/strings/lists/CStringList.H @@ -140,6 +140,9 @@ public: // Access + //- True if the size is zero. + inline bool empty() const; + //- Return the number of C-strings (ie, argc) inline int size() const; @@ -159,6 +162,7 @@ public: //- The flattened character content, with interspersed nul-chars inline const char* data() const; + // Edit //- Clear contents and free memory diff --git a/src/OpenFOAM/primitives/strings/lists/CStringListI.H b/src/OpenFOAM/primitives/strings/lists/CStringListI.H index 6b9efee13a..814160c91a 100644 --- a/src/OpenFOAM/primitives/strings/lists/CStringListI.H +++ b/src/OpenFOAM/primitives/strings/lists/CStringListI.H @@ -111,6 +111,12 @@ inline void Foam::CStringList::clear() } +inline bool Foam::CStringList::empty() const +{ + return !argc_; +} + + inline int Foam::CStringList::size() const { return argc_; diff --git a/src/thermophysicalModels/radiation/radiationModels/solarLoad/faceShading/faceShading.C b/src/thermophysicalModels/radiation/radiationModels/solarLoad/faceShading/faceShading.C index fcc79c3449..2c76eeeedf 100644 --- a/src/thermophysicalModels/radiation/radiationModels/solarLoad/faceShading/faceShading.C +++ b/src/thermophysicalModels/radiation/radiationModels/solarLoad/faceShading/faceShading.C @@ -66,7 +66,7 @@ void Foam::faceShading::writeRays Pout<< "cmd: objToVTK " << fName.c_str() << endl; - stringList cmd{"objToVTK", fName, fName.lessExt().ext("vtk")}; + stringList cmd({"objToVTK", fName, fName.lessExt().ext("vtk")}); Foam::system(cmd); }