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

[Bruno Haible]

Kamil Dudka wrote:
> >   a) A non-symlink, non-directory. Here acl_extended_file_nofollow and
> >      acl_extended_file are equivalent.
> 
> If I understand this, you expect non-directories cannot be mount points, thus 
> the call cannot trigger the mount, right?

That's what I was assuming, yes. My kernel knowledge is rusty, though. Is it
now possible to use regular files as mount points?

> >   c) A mount point.
> >   f) A symlink to a mount point, with dereferencing (stat()).
>
> The behavior of ls wrt. tregerring mounts is implementation defined, isn't it?

Yes. But if you trigger an autofs mount in case f), it will be seen as a
bug, in the same way as it was considered a bug in case c).

> > So I'm asking to change
> >
> >     ret = acl_extended_file_wrap (name);
> >
> > to
> >
> >     if (S_ISDIR (sb->st_mode))
> >       ret = acl_extended_file_wrap (name);
> >     else
> >       ret = acl_extended_file (name);
> 
> Is optimization the only purpose of this change?  I would then prefer to get 
> it working first and optimize it later on.

I'm trying to explain to you that
  - your patch is introducing a performance regression in case a), which is
    the most frequent case,
  - your patch does not completely work yet: case f).

> The last patch does not break anything that worked before, does it?

It degrades the performance of "ls -l" in the most frequent case.

> Case f) has never worked, nobody complained.  Partial fix is better than no 
> fix.

I had the impression that you wanted to fix "ls -l", not fix some bugs and
leave others open.

And I wanted to help coreutils by stating which are the 9 cases which IMO
should have a test case, each.

> >   - Or use code equivalent to canonicalize_filename_mode(CAN_EXISTING).
> 
> I am personally not intersted in changing something that works unless there 
> is 
> a user that needs the change actually.

Whereas I consider an inconsistency between case c) and case f) to be a bug,
even if no user has reported it so far.

Bruno