bug-gnulib
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] file-has-acl: revert unintended change in behavior of ls -L


From: Bruno Haible
Subject: Re: [PATCH] file-has-acl: revert unintended change in behavior of ls -L
Date: Mon, 3 Oct 2011 15:45:36 +0200
User-agent: KMail/1.13.6 (Linux/2.6.37.6-0.5-desktop; KDE/4.6.0; x86_64; ; )

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, when it is not
necessary.

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).

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??

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.
  2) see a test case added to gnulib or coreutils.

If you cannot come up with such a test case, I suggest to revert the patch
of 2011-07-22.

Bruno



reply via email to

[Prev in Thread] Current Thread [Next in Thread]