[filesystem] symbolic_link_exists naming AND is_file
I'm writing a small program with filesystem and I needed to check for symbolic links so I can avoid processing them. I noticed the symbolic_link_exists method and, like all good programmers, didn't read the docs b/c I was sure it didn't do what I needed. So I set about writing a function that would and when I looked at the filesystem implementation I realized I was wrong. I was thinking that symbolic_link_exits would check that a symbolic link points to a valid file -- essentially following the symlink. Any reason why this funtion isn't called is_symlink to match the is_directory call? Also, in the code I'm writing I need to only process regular files. So it would be nice to write: if (is_file(my_path)) { //do stuff } where is_file is defined as follows: // equivalent of if (-d path) in perl // returns !is_directory(p) && !is_symlink(p); bool is_file(fs::path p); Or maybe it could be is_regular_file or is_plain_file if we want to be verbose. Seems like this would be handy to have in the filesystem lib. I can probably be talked into supplying patches ;-) Thoughts? Jeff
I'm writing a small program with filesystem and I needed to check for symbolic links so I can avoid processing them. I noticed the symbolic_link_exists method and, like all good programmers, didn't read the docs b/c I was sure it didn't do what I needed. So I set about writing a function that would and when I looked at the filesystem implementation I realized I was wrong. I was thinking that symbolic_link_exits would check that a symbolic link points to a valid file -- essentially following the symlink. Any reason why this funtion isn't called is_symlink to match
At 09:15 PM 3/19/2005, Jeff Garland wrote: the
is_directory call?
Also, in the code I'm writing I need to only process regular files. So it would be nice to write:
if (is_file(my_path)) {
//do stuff }
where is_file is defined as follows: // equivalent of if (-d path) in perl // returns !is_directory(p) && !is_symlink(p); bool is_file(fs::path p);
Or maybe it could be is_regular_file or is_plain_file if we want to be verbose. Seems like this would be handy to have in the filesystem
Here is the rationale given in the documentation at http://www.boost.org/libs/filesystem/doc/operations.htm#symbolic_link_exists Rationale: The function does not throw if ph is not present, and is accordingly named symbolic_link_exists rather than is_symbolic_link. Non-throwing behavior permits testing for all four possible conditions: * ph not present: !exists(ph) && !symbolic_link_exists(ph) * ph present and is not a symbolic link: exists(ph) && !symbolic_link_exists(ph) * ph present and is a symbolic link to a non-existent file or directory: !exists(ph) && symbolic_link_exists(ph) * ph present and is a symbolic link to an existing file or directory: exists(ph) && symbolic_link_exists(ph) In other words, the current naming and semantics (not throwing), is both logically consistent and allows all possible conditions to be tested for. I guess you could make an argument that the name is_symbolic_link() with non-throwing semantics would be more satisfying in some sense even if less logically consistent. lib. I
can probably be talked into supplying patches ;-)
Thoughts?
The original rationale for not supplying is_file() was (1) concern over devices, which are treated as files in some operating systems, but not in other operating systems, and (2) minimalism, since it was trivial to code !directory() directly. That was before the issue of symbolic links came up, so it might be worth revisiting. But why do you add "&& !is_symlink(p)"? My expectation for a function named is_file() would be that it be "deep" and ignore the fact that the file was reached via a symbolic_link. In other works, the implementation would just be { return !directory(ph); // note that this may throw } --Beman
On Sun, 20 Mar 2005 20:29:13 -0500, Beman Dawes wrote
At 09:15 PM 3/19/2005, Jeff Garland wrote: Here is the rationale given in the documentation at http://www.boost.org/libs/filesystem/doc/operations.htm#symbolic_link_exists
I guess I really should read the documentation...it's bad when you get caught not reading the manual twice in one day ;-)
Rationale: The function does not throw if ph is not present, and is accordingly named symbolic_link_exists rather than is_symbolic_link. Non-throwing behavior permits testing for all four possible conditions:
* ph not present: !exists(ph) && !symbolic_link_exists(ph) * ph present and is not a symbolic link: exists(ph) && !symbolic_link_exists(ph) * ph present and is a symbolic link to a non-existent file or directory: !exists(ph) && symbolic_link_exists(ph) * ph present and is a symbolic link to an existing file or directory: exists(ph) && symbolic_link_exists(ph)
In other words, the current naming and semantics (not throwing), is both logically consistent and allows all possible conditions to be tested for.
Ok, I see. It's a bit non-intuitive that exists returns false when the symbolic link is there, but it does work as documented.
I guess you could make an argument that the name is_symbolic_link() with non-throwing semantics would be more satisfying in some sense even if less logically consistent.
I think it would be a better name. It basically tells you if the file is a symbolic link -- it doesn't tell you that the target of the link exists which is what I thought from the name.
The original rationale for not supplying is_file() was (1) concern over devices, which are treated as files in some operating systems, but not in other operating systems, and (2) minimalism, since it was trivial to code !directory() directly.
That was before the issue of symbolic links came up, so it might be worth revisiting.
Yep...
But why do you add "&& !is_symlink(p)"? My expectation for a function named is_file() would be that it be "deep" and ignore the fact that the file was reached via a symbolic_link. In other works, the implementation would just be
{ return !directory(ph); // note that this may throw }
No I don't want the function to be 'deep' because I'm changing the names of the only the actual files and I want to sidestep the symlinks in the directory. Here's how I implemented it: BOOST_FILESYSTEM_DECL bool is_file( const path & ph ) { # ifdef BOOST_POSIX struct stat path_stat; if ( ::lstat( ph.native_directory_string().c_str(), &path_stat ) != 0 ) boost::throw_exception( filesystem_error( "boost::filesystem::is_file", ph, fs::detail::system_error_code() ) ); return S_ISREG( path_stat.st_mode ); # else //No such thing as a symbolic link on windows so it's a file if its not a directory return !is_directory(ph); # endif } Now we certainly could implement this on top of the existing library functions, but it would require 2 calls to stat/lstat. That combined with the fact that both C and perl have a special function is enough to convince me of it's need. Finally, I tried out this implementation against one of those device files and it reports 'exists' true and 'is_file' false -- which is makes sense since it's a device not a file... If you think it's a good idea I can just check in the change. Jeff
At 09:28 PM 3/20/2005, Jeff Garland wrote:
In other words, the current naming and semantics (not throwing), is both logically consistent and allows all possible conditions to be tested for.
Ok, I see. It's a bit non-intuitive that exists returns false when the symbolic link is there, but it does work as documented.
I guess you could make an argument that the name is_symbolic_link() with non-throwing semantics would be more satisfying in some sense even if less logically consistent.
I think it would be a better name. It basically tells you if the file is a symbolic link -- it doesn't tell you that the target of the link exists which is what I thought from the name.
I'm inclined to agree with you. Let me think about it a bit more.
The original rationale for not supplying is_file() was (1) concern over devices, which are treated as files in some operating systems, but not in other operating systems, and (2) minimalism, since it was trivial to code !directory() directly.
That was before the issue of symbolic links came up, so it might be worth revisiting.
Yep...
But why do you add "&& !is_symlink(p)"? My expectation for a function named is_file() would be that it be "deep" and ignore the fact that the file was reached via a symbolic_link. In other works, the implementation would just be
{ return !directory(ph); // note that this may throw }
No I don't want the function to be 'deep' because I'm changing the names of the only the actual files and I want to sidestep the symlinks in the directory.
Here's how I implemented it:
BOOST_FILESYSTEM_DECL bool is_file( const path & ph ) { # ifdef BOOST_POSIX struct stat path_stat; if ( ::lstat( ph.native_directory_string().c_str(), &path_stat ) != 0 ) boost::throw_exception( filesystem_error( "boost::filesystem::is_file", ph, fs::detail::system_error_code() ) ); return S_ISREG( path_stat.st_mode ); # else //No such thing as a symbolic link on windows so it's a file if its
not a directory return !is_directory(ph); # endif }
Now we certainly could implement this on top of the existing library functions, but it would require 2 calls to stat/lstat. That combined with the fact that both C and perl have a special function is enough to convince me of it's need. Finally, I tried out this implementation against one of those device files and it reports 'exists' true and 'is_file' false -- which is makes sense since it's a device not a file...
If you think it's a good idea I can just check in the change.
I'd prefer you didn't just yet. I'm still cogitating on it, and in any case would probably want to hold new functionality for the I18N branch, which I'm hoping to commit within the next day or two. --Beman
On Mon, 21 Mar 2005 18:21:43 -0500, Beman Dawes wrote
Now we certainly could implement this on top of the existing library functions, but it would require 2 calls to stat/lstat. ...snip... If you think it's a good idea I can just check in the change.
I'd prefer you didn't just yet. I'm still cogitating on it, and in any case would probably want to hold new functionality for the I18N branch, which I'm hoping to commit within the next day or two.
Ok no problem -- the change was basically in the last email anyway -- it's really trivial... Jeff
participants (2)
-
Beman Dawes -
Jeff Garland