using filesystem read_link to test if sym link already created. #417
Conversation
nickhuang99
commented
Jun 21, 2024
nickhuang99
commented
- using std::filesystem's read_link.
- This is an improvement so only test when failure to avoid expensive system call.
- Hopefully this avoid race condition.
nickhuang99
force-pushed
the
testSymlinkAfterFail
branch
from
ba78af8 to
67975bf
Compare
mgautierfr
left a comment
mgautierfr
left a comment
There was a problem hiding this comment.
On code side, only a small change needed.
On the functionality itself, I'm a bit reluctant as we should not have this kind of situation. If symlinks already exist, there is a lot of probability that contents already exist too. Only checking for symlink is useless here. And if we overwrite content/symlink we should probably have a option at command line level to allow it.
But anyway, even with my concerns, I don't see how this can harm so I will not block this PR.
| write_to_file(directory + SEPARATOR, outputPath, content.c_str(), content.size()); | ||
| } | ||
|
|
||
| inline static bool testSymLink(const std::string& sym, const std::string& target) { |
There was a problem hiding this comment.
testSymlink convey very few information. (What is tested ?)
It would be better to rename it to checkSymlinkTarget or symlinkPointsTo.
|
@nickhuang99 Have you abandoned this PR? Can you please answer feedback from @mgautierfr ? |
kelson42
force-pushed
the
testSymlinkAfterFail
branch
from
67975bf to
b1b1ad0
Compare
3 participants
