Re: [boost] Filesystem: remove_all should not follow symlinks

From: Jeremy Maitin-Shepard (jbms_at_[hidden])
Date: 2003-11-26 14:31:11

Walter Landry <wlandry_at_[hidden]> writes:

> "Peter Dimov" <pdimov_at_[hidden]> wrote:
>> Walter Landry wrote:
>> > Beman Dawes <bdawes_at_[hidden]> wrote:
>> >> Explicit dereferencing would be awfully cumbersome. People using
>> >> O/S's which natively support symbolic links would likely view it as
>> >> a giant step backward for them. Plus there would be interoperability
>> >> problems - programs using Boost.Filesystem wouldn't work the way
>> >> other programs worked.
>> >
>> > One solution is to have an extra argument which indicates whether to
>> > resolve symlinks, and have the default be yes.  Something like
>> >
>> >   bool exists(const path &ph, bool resolve_symlinks=true)
>> >
>> > Then remove_all would use exists(ph,false).
>> 
>> I don't see how this fixes things. remove_all should work on a symlinked
>> path. It should not _descend into_ symlinks.
>> 
>> bool is_directory( const path &ph, bool symlink_allowed = true )
>> 
>> is a solution, but an explicit "is_symbolic_link" seems better.

> Right.  Not thinking straight.  Having an is_symbolic_link() function
> is certainly still required.  However, I think that my proposed syntax
> would resolve the problem of whether a particular function should
> resolve symlinks or not.

As additional property functions (like last_write_time) are added, it
will be necessary to add such checks to each function.  Furthermore, I
think the resultant syntax is much less clear than the syntax I
proposed, namely, a "dereference" or "resolve" function (or operator).

At least to me, exists(*ph) or exists(resolve(ph)) or
exists(dereference(ph)) is _much_ clearer than exists(ph, true),
and similarly, last_write_time(*ph) or last_write_time(resolve(ph))
than last_write_time(ph, true).

>> I'd also consider something along the lines of
>> 
>> recursive_directory_iterator it(ph, depth_first_traversal | return_files
>> | return_directories | follow_symlinks | detect_cycles), end;
>> 
>> std::for_each(it, end, fs::remove);

In order to make this work, it would be necessary to provide a DFS that
called the predicate while `backtracking,' rather than (as is typical)
when it first reaches a node.

> How about just extending directory_iterator?

This seems reasonable.

-- 
Jeremy Maitin-Shepard