Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix undefined behavior when getting the current working directory from std::filesystem #728

Merged
merged 3 commits into from
Oct 17, 2024

Conversation

giacomofiorin
Copy link
Member

Adds two functions colvarproxy_io::get_current_work_dir() and colvarproxy_io::join_paths() to replace the code currently used in colvarbias_meta.cpp. It's possible that these may be removed again sometime in the future.

Fixes #726

@giacomofiorin giacomofiorin added the bugfix To be used only in PRs label Oct 12, 2024
@giacomofiorin giacomofiorin requested a review from HubLot October 12, 2024 02:35
Copy link

@al42and al42and left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few nitpicks

@giacomofiorin
Copy link
Member Author

Thanks @HubLot and @al42and!

I removed the virtual for the new functions, since it is indeed unlikely that we need to add a special case before we are able to assume std::filesystem everywhere. (We do, however, need to do file I/O differently for NAMD, which has its own class to replace std::ofstream).

As for static, they could definitely be but that class doesn't have any other statics, so I preferred to keep it consistent with other member functions.

@giacomofiorin giacomofiorin merged commit b5d8709 into master Oct 17, 2024
14 checks passed
@giacomofiorin giacomofiorin deleted the fix-getcwd branch October 17, 2024 23:31
acmnpv pushed a commit to gromacs/gromacs that referenced this pull request Oct 25, 2024
All changes are limited to the copy of the library in `src/external/colvars`.

The following is a list of relevant pull requests (bugfixes only) in the library's repository:

- 728 Fix undefined behavior when getting the current working directory from std::filesystem Colvars/colvars#728 (@giacomofiorin)
- 724 Fix gradients and metric functions of distanceDir Colvars/colvars#724 (@giacomofiorin)
- 715 Add missing rotation in orientation component Colvars/colvars#715 (@giacomofiorin)
- 713 fix: try to solve #87 for non-scala components Colvars/colvars#713 (@HanatoK)
- 706 BUGFIX for Segmentation fault in colvarbias_meta::calc_energy() with useGrids off Colvars/colvars#706 (@alphataubio)
- 694 More robust condition to decide when biases run on thread 0 Colvars/colvars#694 (@giacomofiorin)
- 675 Fix initialization of histogram output files and move it to the right place Colvars/colvars#675 (@giacomofiorin)

Authors: @alphataubio, @giacomofiorin, @HanatoK
jhenin added a commit to Colvars/lammps that referenced this pull request Dec 16, 2024
- 759 min_image fix; Saves long runs from crashes;
  Colvars/colvars#759 (@PolyachenkoYA)

- 728 Fix undefined behavior when getting the current working directory from std::filesystem
  Colvars/colvars#728 (@giacomofiorin)

- 724 Fix gradients and metric functions of distanceDir
  Colvars/colvars#724 (@giacomofiorin)

- 715 Add missing rotation in orientation component
  Colvars/colvars#715 (@giacomofiorin)

- 713 fix: try to solve lammps#87 for non-scala components
  Colvars/colvars#713 (@HanatoK)

- 706 BUGFIX for Segmentation fault in colvarbias_meta::calc_energy() with useGrids off
  Colvars/colvars#706 (@alphataubio)

- 701 Do not try accessing LAMMPS proxy object before allocating it
  Colvars/colvars#701 (@giacomofiorin)

Authors: @alphataubio, @giacomofiorin, @HanatoK, @PolyachenkoYA
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix To be used only in PRs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Inconsistent GETCWD in src/colvarbias_meta.cpp can cause underfined behavior
3 participants