This is the mail archive of the cygwin-patches@cygwin.com mailing list for the Cygwin project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [Patch]: path.cc


At 11:01 PM 4/3/2004 -0500, Christopher Faylor wrote:
>On Sat, Apr 03, 2004 at 09:49:40PM -0500, Pierre A. Humblet wrote:

>Also, there is a problem in execution:
<snip>

>I believe that this is due to your removal of the normalize stuff from
>mount_info::conv_to_win32_path.  

It turns out that the normalized stuff was never called in 
mount_info::conv_to_win32_path and so that's really old cruft.
It was always called with no_normalize.
The problem was that need_directory is now checked after calling
normalize_posix_path, which was keeping /alice/ intact but changing
/alice/. into /alice  , so that need_directory wasn't set.
That's fixed by removing a line. Now /alice/. becomes /alice/
then need_directory is set and finally /alice/ becomes /alice  

Hmm, that's embarrassing considering I started this after the
"final dot discussion"  in 
http://cygwin.com/ml/cygwin/2004-03/msg01386.html

>Also, maybe I'm missing something, but it seems like your change to
>pathnmatch is not necessarily an optimization.  It depends on the path.

Right.

>If one is often comparing paths that differ in the last len1-n
>characters then doing the isdirsep, etc.  checks makes sense.  It only
>seems like it would be an operation if you were routinely compared paths
>like "c:\cygwin" and "c:\cygwinfoo" which is rarely the case, IMO.

Absolutely right. It's an optimization if the probability that
"isdirsep() detects a mismatch" is greater than 
"cost of executing isdirsep() " / "cost of executing the pathnmatch".
The latter involves two function calls while isdirsep() is a simple macro,
so my gut feeling is that the change is good.
 
>Except that old symlinks that use EA would be slightly slower, right?
>
>I agree that it is not worth keeping this in the inner loop.  I have
>always wanted to cache this data, too, rather than continually looking
>it up.

Done.

>So, sorry, this patch isn't yet ready for prime time.

Here we go again, from a clean sandbox.

>Can you break it down into smaller pieces, maybe?

It's like pulling a thread... At this point I would need to undo some
changes and retest. The next one will be shorter.
Perhaps I should not have added the removal of fs.update(), but given
that we have discussed it...

Pierre

2004-04-04  Pierre Humblet <pierre.humblet@ieee.org>

	* path.cc (path_prefix_p): Optimize test order.
	(normalize_posix_path): Add "tail" argument and set it. Always have a
	final slash for directories. Pass 3rd argument to normalize_win32_path.
	(path_conv::check): Pass tail to normalize_posix_path. Set need_directory
	and remove final slash after that call. Remove last argument to
	mount_table->conv_to_win32_path(). Remove noop dostail check. Remove
	fs.update() from inner loop. Improve tail finding search.
	(normalize_win32_path): Add and set tail argument.
	(mount_item::build_win32): Avoid calling strcpy.
	(mount_info::conv_to_win32_path): Remove third argument and simplify
	because the source is normalized. Keep /proc path in Posix form.
	Call win32_device_name() only once.
	(mount_info::conv_to_posix_path): Add and use 3rd argument to 
	normalize_win32_path to avoid calling strlen. 
	(cwdstuff::set): Add 3rd argument to normalize_posix_path and remove final
	slash if any. 
	* shared_info.h (mount_info::conv_to_win32_path): Remove last argument
	in declaration.
 

Attachment: path.cc.diff
Description: Text document


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]