boost::filesystem::remove() throws on some symlinks
Hello If one creates a symlink like this "ln -s sym sym" (to itself symlink), bfs::exists("sym") will throw because the documentation defines it to do so (and I have nothing against it doing so, although in my programs because of this I am forced to just use "lstat()" and I can't replace using "lstat()" to check if something exists with bfs::exists). However, I do think that bfs::remove("sym") throwing for the same reason (because it calls upon bfs::exists()) is the wrong thing to do. The source around bfs::remove() implementation even try to solve the issue of "dangling symlinks" as they call it by having an exception in case bfs::exists returns false and the target path points to a dangling symlink and still tries to remove it. Unfortunetly, exists will throw in my example of a symlink (not even dangling technically speaking :) ) even tho a remove on that kind of symlink should work just fine (it does work in shell or just doing std::remove()). Is this a bug? If so, I should put it in the tracker? Thanks! -- Mihai RUSU Email: dizzy@roedu.net "Linux is obsolete" -- AST
On Friday 30 May 2008 12:52:29 you wrote:
Is this a bug? If so, I should put it in the tracker?
In the mean time as one of my coleagues has found that remove_all is also affected by the issue (once because it also does a broken bfs::exists() check first and second because remove_all_aux() calls bfs::remove() to remove elements) and I noticed that remove and especially remove_all implementations are not written with efficency in mind, ie remove_all cals indirectly lstat/fstat about 4 times per each element that will be deleted: - one "lstat" from is_symlink - a "fstat" from is_directory - a "fstat" from remove() bfs::exists call - in case the file is a directory another "fstat" from directory_iterator ctor - in case the removed file is a dangling symlink another "lstat" from is_symlink from remove()) instead of doing it once and reusing the returned value. I have reworked remove and remove_all functions in boost 1.35.0 (I also have a patch for 1.34.1 for who is interested since we have some software still using it) to not have the reported bug and be more efficient "lstat/fstat" wise. Patch attached. PS: the resulting code still has one redundant "stat" because of directory_iterator ctor, if one wants to eliminate that too just have a directory_iterator ctor version that takes a file_status argument instead of invoking itself (indirectly) "stat" to query that information -- Mihai RUSU Email: dizzy@roedu.net "Linux is obsolete" -- AST
dizzy wrote:
Hello
If one creates a symlink like this "ln -s sym sym" (to itself symlink), bfs::exists("sym") will throw because the documentation defines it to do so (and I have nothing against it doing so, although in my programs because of this I am forced to just use "lstat()" and I can't replace using "lstat()" to check if something exists with bfs::exists).
Interesting! That's a case I never considered.
However, I do think that bfs::remove("sym") throwing for the same reason (because it calls upon bfs::exists()) is the wrong thing to do. The source around bfs::remove() implementation even try to solve the issue of "dangling symlinks" as they call it by having an exception in case bfs::exists returns false and the target path points to a dangling symlink and still tries to remove it. Unfortunetly, exists will throw in my example of a symlink (not even dangling technically speaking :) ) even tho a remove on that kind of symlink should work just fine (it does work in shell or just doing std::remove()).
Is this a bug?
Probably.
If so, I should put it in the tracker?
Yes, please do. Please mention what operating system you are using. I assume it is Linux, but it always helps to know for sure with filesystem issues. Thanks, --Beman
On Friday 30 May 2008 15:11:04 Beman Dawes wrote:
dizzy wrote:
Hello
If one creates a symlink like this "ln -s sym sym" (to itself symlink), bfs::exists("sym") will throw because the documentation defines it to do so (and I have nothing against it doing so, although in my programs because of this I am forced to just use "lstat()" and I can't replace using "lstat()" to check if something exists with bfs::exists).
Interesting! That's a case I never considered.
Yeah, we stumbled on it by accident as it appears one of the users of the software had such a thing :) Either way it is something very possible on probably any POSIX system.
If so, I should put it in the tracker?
Yes, please do.
Please mention what operating system you are using. I assume it is Linux, but it always helps to know for sure with filesystem issues.
I have reported it, thanks for your answer. I thought of another thing too, maybe besides "exists(path)" there should be a "symlink_exists(path)" (basically what symlink_status() is to status()), instead of doing status() as exists() does, symlink_exists() could do symlink_status() and check if a symlink OR a another file exists. Then in any code where I need to check if something exists that's possibly a symlink (that resolves correctly or not) I could use symlink_exists(). remove/remove_all are other examples that instead of exists() could use this new symlink_exists(). The only problem I see with the symlink_exists() is that the old filesystem API had something named somewhat similar that was used instead of the current "is_symlink()", I'm speaking about the deprecated "symbolic_link_exists()". For people used to it, "symlink_exists()" with different semantics might confuse them (tho it should not break existent code that still uses "symbolic_link_exists" of course). What do you think? -- Mihai RUSU Email: dizzy@roedu.net "Linux is obsolete" -- AST
participants (2)
-
Beman Dawes -
dizzy