Bug#336560: vim loops forever in readlink on startup when opening symlinked file with two parent directories of the same name

[James Vega]

On Tue, Nov 01, 2005 at 05:08:57PM +0100, Stefano Zacchiroli wrote:
> On Mon, Oct 31, 2005 at 09:29:23PM -0500, James Vega wrote:
> > The attached patch fixes the bug.

Actually, I think I was this is incorrect.  It looks like the problem is
more complex than I thought.  You were on the right path, but were
missing one thing.  Here is a breakdown of the problem:

# Using the bug reporter's situation:
# /tmp/vim-bug/vim-bug/subdir/testfile
# /tmp/vim-bug/subdir/testfile -> ../vim-bug/subdir/testfile
# cwd = "/tmp/vim-bug"

    void
resolvesymlink(fname, buf, bufsiz)
    char_u	*fname, *buf;
    size_t	bufsiz;
{

# fname = "subdir/testfile" ("/tmp/vim-bug/subdir/testfile")

    char_u	tmp[PATH_MAX];

    if (fname == NULL)
	return;

# I haven't taken a deep look at the code that uses buf after
# resolvesymlink is called, but we may want to have resolvesymlink
# return readlink's return code so we know how long the string in buf
# is.

# readlink call returns 26
# buf = "../vim-bug/subdir/testfile" ("/tmp/vim-bug/../vim-bug/subdir/testfile")

    if (readlink((char *) fname, (char *) buf, bufsiz) == -1)
	STRCPY(buf, fname); /* not a symlink: return fname unmodified */
    else
    {	/* symlink: recursively expand */

# Before we recursively expand, we need to check whether buf is an
# absolute or relative path.  If it is a relative path, we should
# convert it to an absolute path.  This would probable be better as
# (possibly modified to retain the readlink return value):
#
# do {
#	STRCPY(tmp, buf);
#	some_convert_to_absolute_path_function(fname, tmp);
# } while (readlink((char *) tmp, (char *) buf, bufsiz) != -1);

	STRCPY(tmp, buf);

# tmp = "../vim-bug/subdir/testfile" ("/tmp/vim-bug/../vim-bug/subdir/testfile")
# buf = "../vim-bug/subdir/testfile" ("/tmp/vim-bug/../vim-bug/subdir/testfile")
#
# buf should be adjusted for the fact that it is a relative link based
# on being in "/tmp/vim-bug/subdir" when you dereference it.  We're
# instead dereferencing it from "/tmp/vim-bug" which means we perform
# readlink on the same file.
# Since it isn't, we get stuck dereferencing the same symlink.

	while (readlink((char *) tmp, (char *) buf, bufsiz) != -1)
	    STRCPY(tmp, buf);
    }
}

> > My only concern is what we should do
> > if EINVAL isn't what errno is set to[1].  Is that possible in the way
> > the code is called?
> 
> I would say that an "assertion failed" like exit would be appropriate in
> any of the cases errno != EINVAL. Nonetheless one of them is the one
> that should happen in the case described by the bugreport, isn't it?
> Which one does actually happen in that case?

Yes, we should get an EINVAL when we finally call readlink on an actual
file instead of a symlink.

I can work on a patch that does what I outlined above.  I'll post it
here when I'm finished, as my C skills are a little rusty.  Peer review
would be good.  :)

James
-- 
GPG Key: 1024D/61326D40 2003-09-02 James Vega <jamessan at jamessan.com>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: not available
Url : http://lists.alioth.debian.org/pipermail/pkg-vim-maintainers/attachments/20051101/c9cfd44b/attachment-0001.pgp