Skip to content

Commit

Permalink
Fix undefined behavior when getting CWD from std::filesystem
Browse files Browse the repository at this point in the history
  • Loading branch information
giacomofiorin committed Oct 11, 2024
1 parent 37fb8df commit 8027ed3
Show file tree
Hide file tree
Showing 3 changed files with 69 additions and 38 deletions.
44 changes: 7 additions & 37 deletions src/colvarbias_meta.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -11,24 +11,6 @@
#include <iomanip>
#include <algorithm>

// Define function to get the absolute path of a replica file
#if defined(_WIN32) && !defined(__CYGWIN__)
#include <direct.h>
#define GETCWD(BUF, SIZE) ::_getcwd(BUF, SIZE)
#define PATHSEP "\\"
#else
#include <unistd.h>
#define GETCWD(BUF, SIZE) ::getcwd(BUF, SIZE)
#define PATHSEP "/"
#endif

#ifdef __cpp_lib_filesystem
// When std::filesystem is available, use it
#include <filesystem>
#undef GETCWD
#define GETCWD(BUF, SIZE) (std::filesystem::current_path().string().c_str())
#endif

#include "colvarmodule.h"
#include "colvarproxy.h"
#include "colvar.h"
Expand Down Expand Up @@ -1739,29 +1721,17 @@ int colvarbias_meta::setup_output()

if (comm == multiple_replicas) {

// TODO: one may want to specify the path manually for intricated filesystems?
char *pwd = new char[3001];
if (GETCWD(pwd, 3000) == nullptr) {
if (pwd != nullptr) { //
delete[] pwd;
}
return cvm::error("Error: cannot get the path of the current working directory.\n",
COLVARS_BUG_ERROR);
}

auto const pwd = cvm::main()->proxy->get_current_work_dir();
replica_list_file =
(std::string(pwd)+std::string(PATHSEP)+
this->name+"."+replica_id+".files.txt");
cvm::main()->proxy->join_paths(pwd, this->name + "." + replica_id + ".files.txt");
// replica_hills_file and replica_state_file are those written
// by the current replica; within the mirror biases, they are
// those by another replica
replica_hills_file =
(std::string(pwd)+std::string(PATHSEP)+
cvm::output_prefix()+".colvars."+this->name+"."+replica_id+".hills");
replica_state_file =
(std::string(pwd)+std::string(PATHSEP)+
cvm::output_prefix()+".colvars."+this->name+"."+replica_id+".state");
delete[] pwd;
replica_hills_file = cvm::main()->proxy->join_paths(
pwd, cvm::output_prefix() + ".colvars." + this->name + "." + replica_id + ".hills");

replica_state_file = cvm::main()->proxy->join_paths(
pwd, cvm::output_prefix() + ".colvars." + this->name + "." + replica_id + ".state");

// now register this replica

Expand Down
57 changes: 56 additions & 1 deletion src/colvarproxy_io.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,20 @@
// Colvars repository at GitHub.

// Using access() to check if a file exists (until we can assume C++14/17)
#if !defined(_WIN32) || defined(__CYGWIN__)
#if defined(_WIN32) && !defined(__CYGWIN__)
#include <direct.h>
#else
#include <unistd.h>
#endif

#if defined(_WIN32)
#include <io.h>
#endif

#ifdef __cpp_lib_filesystem
#include <filesystem>
#endif

#include <cerrno>
#include <cstdio>

Expand Down Expand Up @@ -64,6 +71,54 @@ int colvarproxy_io::set_frame(long int)
}


std::string colvarproxy_io::get_current_work_dir() const
{
#ifdef __cpp_lib_filesystem

return std::filesystem::current_path().string();

#else

// Legacy code
size_t constexpr buf_size = 3001;
std::string cwd;
char buf[buf_size];

#if defined(_WIN32) && !defined(__CYGWIN__)
char *getcwd_result = ::_getcwd(buf, buf_size);
#else
char *getcwd_result = ::getcwd(buf, buf_size);
#endif

if (getcwd_result == nullptr) {
cvm::error("Error: cannot read the current working directory.\n", COLVARS_INPUT_ERROR);
return std::string("");
}

return std::string(getcwd_result);
#endif
}


std::string colvarproxy_io::join_paths(std::string const &path1, std::string const &path2) const
{
#ifdef __cpp_lib_filesystem

return (std::filesystem::path(path1) / std::filesystem::path(path2)).string();

#else

// Legacy code
#if defined(_WIN32) && !defined(__CYGWIN__)
return (path1 + "\\" + path2);
#else
return (path1 + "/" + path2);
#endif

#endif
}


int colvarproxy_io::backup_file(char const *filename)
{
// Simplified version of NAMD_file_exists()
Expand Down
6 changes: 6 additions & 0 deletions src/colvarproxy_io.h
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,12 @@ class colvarproxy_io {
// Returns error code
virtual int set_frame(long int);

/// Get the current working directory of this process
virtual std::string get_current_work_dir() const;

/// Join two paths using the operating system's path separation
virtual std::string join_paths(std::string const &path1, std::string const &path2) const;

/// \brief Rename the given file, before overwriting it
virtual int backup_file(char const *filename);

Expand Down

0 comments on commit 8027ed3

Please sign in to comment.