On Mon October 3 2011 15:45:36 Bruno Haible wrote: > Kamil Dudka wrote: > > The commit 95f7c57 introduced an unintended change in behavior of ls -L. > > I am attaching a patch that restores the old behavior. > > This patch introduces an additional lstat() system call
Yes. > , when it is not necessary. I disagree. ls -L does not do lstat(). > Recall that when file_has_acl is called, it gets the result of stat() or > lstat() passed through the 'sb' argument. If it's the result of stat(), > the user is interested in the symlink's target, so the code should call > acl_extended_file(). If it's the result of lstat(), the user is interested > in the symlink itself, so the code should call > acl_extended_file_nofollow(). > > So there are 3 cases: > a) A non-symlink, hence acl_extended_file_nofollow and acl_extended_file > are equivalent. > b) A symlink, with dereferencing (stat()). Must call acl_extended_file. > c) A symlink, without dereferencing (lstat()). Must call > acl_extended_file_nofollow. > > You are using lstat() to distinguish case a) from b)+c). But it is also > possible to inspect *sb, to distinguish case c) from a)+b). How exactly? I do not get it. > In fact, the code already does this, at the beginning of the function > file_has_acl. So in case c) the big compound statement is skipped, and the > function immediately does a "return 0". The patch that you proposed on > 2011-04-06 and committed on 2011-07-22 therefore can only have an effect in > the cases a) and b). In the case b) you have introduced a regression, > that's what you've discovered now. And in the case a) - when the file is > not a symlink: are you saying in <https://bugzilla.redhat.com/720325> that > calling acl_extended_file on a non-symlink will trigger autofs mounts?? Yes, the getxattr syscall triggers it. lgetxattr does not. > So, before adding more complexity to the patch of 2011-07-22, I would like > to > 1) understand better which file system operations triggered the autofs > mounts, I can't really beleive that calling getxattr on a non-symlink > will trigger autofs mounts. That's the information I was given. As a user-space hacker, I cannot help with providing more details about the kernel internals. > 2) see a test case added to gnulib or coreutils. A while ago, Jim wrote a test-case for coreutils that catches exactly the bug that the original patch introduced. > If you cannot come up with such a test case, I suggest to revert the patch > of 2011-07-22. Why? AFAIK, there has never been a test-case for the ACL detection in ls. Kamil
