[Top][All Lists]
[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:02:20 +0200 |
User-agent: |
KMail/1.13.6 (Linux/2.6.37.6-0.5-desktop; KDE/4.6.0; x86_64; ; ) |
Kamil Dudka wrote:
> Thanks in advance for considering the patch!
> +#if (HAVE_ACL_EXTENDED_FILE) /* Linux */
Unnecessary parentheses.
> +acl_extended_file_wrap (char const *name)
The function name should explain the semantics of the function. The fact
that it's a wrapper around acl_extended_file is something one can see by
reading the code.
Maybe call it acl_extended_file_optimized?
> +# if HAVE_ACL_EXTENDED_FILE_NOFOLLOW
> +#endif /* HAVE_ACL_EXTENDED_FILE_NOFOLLOW */
Incorrect indentation of the #endif line.
> + /* acl_extended_file_nofollow() uses lgetxattr() in order to prevent
> + unnecessary mounts, but it returns the same result as we already
> + know that NAME is not a symbolic link at this point (modulo the
> + TOCTTOU race condition). */
I have a hard time understanding this comment.
- Why the "but"? The "same result" as what?
- What is the "TOCTTOU race condition"? If it's important, please add a FIXME
and an explanation.
How about this comment, right after the inner #if: 1. Describe the problem.
2. Describe the solution.
/* acl_extended_file() tests whether a file has an ACL. But it can trigger
unnecessary autofs mounts. In newer versions of libacl, a function
acl_extended_file_nofollow() is available that uses lgetxattr() and
therefore does not have this problem. It is equivalent to
acl_extended_file(), except on symbolic links. */
Bruno
Re: [PATCH] file-has-acl: revert unintended change in behavior of ls -L, Jim Meyering, 2011/10/03
Re: [PATCH] file-has-acl: revert unintended change in behavior of ls -L,
Bruno Haible <=
Re: [PATCH] file-has-acl: revert unintended change in behavior of ls -L, Bruno Haible, 2011/10/03