Recent change to file-chase-links has a bug

Recent change to file-chase-links has a bug

[Kenichi Handa]

This change has a bug.

2003-04-08  Richard M. Stallman  <rms@gnu.org>
[...]
	* files.el (file-chase-links): New arg LIMIT.
	After that many iterations, just return what we've got.

Now the code is as below:

(defun file-chase-links (filename &optional limit)
  "Chase links in FILENAME until a name that is not a link.
Unlike `file-truename', this does not check whether a parent
directory name is a symbolic link.
If the optional argument LIMIT is a number,
it means chase no more than that many links and then stop."
  (let (tem (newname filename)
	    (count 0)
	    (max (max limit 100)))
    (while (and (or (null limit) (< count limit))
		(setq tem (file-symlink-p newname)))
      (save-match-data
	(if (= count max)
	    (error "Apparent cycle of symbolic links for %s" filename))

At least, (max limit 100) must be:
	(max (if (numberp limit) limit 0) 100)
Otherwise, for instance, C-x 4 a signals an error.

And, if the docstring is correct, actually the code must be
something like this:

(defun file-chase-links (filename &optional limit)
[...]
  (let (tem (newname filename)
	    (count 0)
	    (max 100)
    (while (and (or (not (numberp limit)) (< count limit))
		(setq tem (file-symlink-p newname)))
      (save-match-data
	(if (= count max)
	    (error "Apparent cycle of symbolic links for %s" filename))

But, I have not yet installed that fix because it seems that
the original line "(max (max limit 100))" suggests some
other intention, e.g., "don't allow a limit less than 100".

---
Ken'ichi HANDA
handa@m17n.org

Re: Recent change to file-chase-links has a bug

[Kai Großjohann]

Kenichi Handa <handa@m17n.org> writes:

> Now the code is as below:
>
> (defun file-chase-links (filename &optional limit)
>   "Chase links in FILENAME until a name that is not a link.
> Unlike `file-truename', this does not check whether a parent
> directory name is a symbolic link.
> If the optional argument LIMIT is a number,
> it means chase no more than that many links and then stop."
>   (let (tem (newname filename)
> 	    (count 0)
> 	    (max (max limit 100)))

Wild guess: Richard meant (or limit 100) instead of (max limit 100).

>     (while (and (or (null limit) (< count limit))
> 		(setq tem (file-symlink-p newname)))
>       (save-match-data
> 	(if (= count max)
> 	    (error "Apparent cycle of symbolic links for %s" filename))

-- 
A preposition is not a good thing to end a sentence with.

Re: Recent change to file-chase-links has a bug

[Kevin Rodgers]

Kai Großjohann wrote:

> Kenichi Handa <handa@m17n.org> writes:
>>Now the code is as below:
>>
>>(defun file-chase-links (filename &optional limit)
>>  "Chase links in FILENAME until a name that is not a link.
>>Unlike `file-truename', this does not check whether a parent
>>directory name is a symbolic link.
>>If the optional argument LIMIT is a number,
>>it means chase no more than that many links and then stop."
>>  (let (tem (newname filename)
>>	    (count 0)
>>	    (max (max limit 100)))
> 
> Wild guess: Richard meant (or limit 100) instead of (max limit 100).


That seems like an awfully conservative default.  How about bumping it up to
1024?

-- 
<a href="mailto:&lt;kevin.rodgers&#64;ihs.com&gt;">Kevin Rodgers</a>

Re: Recent change to file-chase-links has a bug

[Richard Stallman]

    At least, (max limit 100) must be:
	    (max (if (numberp limit) limit 0) 100)
    Otherwise, for instance, C-x 4 a signals an error.

Thanks.  I fixed this.

    And, if the docstring is correct, actually the code must be
    something like this:

That change would make limits greater than 100 not work.