Sender: rich AT phekda DOT freeserve DOT co DOT uk Message-ID: <3E270221.2FA697B@phekda.freeserve.co.uk> Date: Thu, 16 Jan 2003 19:04:01 +0000 From: Richard Dawe X-Mailer: Mozilla 4.77 [en] (X11; U; Linux 2.2.23 i586) X-Accept-Language: de,fr MIME-Version: 1.0 To: djgpp-workers AT delorie DOT com Subject: Re: Rewritten __solve_symlinks to fix bugs [PATCH] References: <3E26F8E8 DOT 2080709 AT mif DOT vu DOT lt> Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit Reply-To: djgpp-workers AT delorie DOT com Hello. Laurynas Biveinis wrote: > > Richard Dawe wrote: > > Hello. > > > > I mentioned a while ago that __solve_symlinks() doesn't work > > with relative symlinks that have too many ".."s, i.e.: when they > > have more ".."s than required to reach the root directory. > > > > (It's possible that it also does not work with symlinks to a file > > in a directory accessed via a symlink to the directory. E.g.: > > > > mkdir foo > > touch foo/bar > > ln -s foo baz > > cat baz/foo > ^^^ > bar ? Yes, sorry for the typo. > I'm not sure I understand the situation you describe here, but previous > implementation should work with commands as you have written them. Like I said, it was probably something I broke, when I was trying to fix the code that's in CVS. > > * How do you know how many directory levels are present? If you have > > a path like /dev/env/DJDIR, then you need to expand it, to find out > > how many directory levels are present. > > > > * How do you collapse ".."s. You could call _fixpath(), but you have to be > > careful - that may over-optimise and remove too many ".."s. You can only > > call _fixpath() on what you have resolved so far. > > Uhm, I guess I could fix these with current implementation. Of course, I > cannot tell for sure without trying first :) > > > * It's a complete rewrite. > > Of course I'm biased here, because my code is being thrown out here :) > but is a complete rewrite justified? Current algorithm has been in > testing for some time already. I don't like throwing code out. It's not something I wanted to do. I know you've put a lot of work in writing & testing that code. If you can fix the current code, then I'll be happy! > It does not have serious design issues or at least the basic algorithm is > identical with yours. My algorithm does some other things. It collapses ".." as much as possible in the resolved path. This is part of reducing "c:/djgpp/../../../..", say, to "c:/", so that the resolving the next part will work OK. DOS doesn't like paths like "c:/djgpp/../../../../../foo" and the old implementation could feed paths like that to __internal_readlink(). (Note that I thought about using _fixpath() in __internal_readlink, to fix the paths passed to _open(), when reading in the symlink. That doesn't help you with the file names returned by __internal_readlink().) Part of the problem with reducing these ".."s is that you need the path to have all special paths (like "/dev/env/DJDIR") to be expanded, so you know what the real directories are. If you don't do this, you could remove, say, DJDIR and end up with a path like "/dev/env", which doesn't make sense. Those are the only real changes over your method. I think the algorithm in my code completely restarts resolution, when it doesn't need to sometimes. E.g.: symlinks with relative paths like "foo" -> "bar", where we could restart resolution at "bar" rather right at the start of the resolved path. I thought about how to avoid that, but I did not code it. > Would you be satisfied if I tried to make current implementation work > with your testcases over a weekend? I'm sorry I have missed discussion > about these bugs (busy times). Yes, I would be very happy. It's quite possible that I've missed some really obvious solution. Like I said, I tried. Pretty hard, in fact. I spent about a day and a half thinking about it, trying out various fixes and testing with fileutils & textutils. > If it turns out that fixing these requires more work than I'm expecting, > then I'm fine with your implementation. OK. Thanks, bye, Rich =] -- Richard Dawe [ http://www.phekda.freeserve.co.uk/richdawe/ ]