[PATCH] better explain functions [q]set-acl.c

[PATCH] better explain functions [q]set-acl.c

[Ondrej Valousek]

Improve comments for both functions

---
 lib/qset-acl.c | 16 +++++++++-------
 lib/set-acl.c  | 13 +++++++------
 2 files changed, 16 insertions(+), 13 deletions(-)

diff --git a/lib/qset-acl.c b/lib/qset-acl.c
index c3442d060f..0ae026f031 100644
--- a/lib/qset-acl.c
+++ b/lib/qset-acl.c
@@ -27,13 +27,15 @@
 #include "acl-internal.h"
 
 
-/* Set the access control lists of a file. If DESC is a valid file
-   descriptor, use file descriptor operations where available, else use
-   filename based operations on NAME.  If access control lists are not
-   available, fchmod the target file to MODE.  Also sets the
-   non-permission bits of the destination file (S_ISUID, S_ISGID, S_ISVTX)
-   to those from MODE if any are set.
-   Return 0 if successful.  Return -1 and set errno upon failure.  */
+/* Set the access control lists of a file to match EXACTLY mode (this might
+   remove inherited ACLs). Note chmod() tends to honor inherited/default
+   ACLs. If DESC is a valid file descriptor, use file descriptor operations 
+   where available, else use filename based operations on NAME.  If access 
+   control lists are not available, fchmod the target file to MODE.  Also 
+   sets the non-permission bits of the destination file 
+   (S_ISUID, S_ISGID, S_ISVTX) to those from MODE if any are set.
+   Return 0 if successful.  On failure, output a diagnostic, set errno and
+   return -1.  */
 
 int
 qset_acl (char const *name, int desc, mode_t mode)
diff --git a/lib/set-acl.c b/lib/set-acl.c
index 9111911864..561ff47824 100644
--- a/lib/set-acl.c
+++ b/lib/set-acl.c
@@ -29,12 +29,13 @@
 #define _(msgid) gettext (msgid)
 
 
-/* Set the access control lists of a file. If DESC is a valid file
-   descriptor, use file descriptor operations where available, else use
-   filename based operations on NAME.  If access control lists are not
-   available, fchmod the target file to MODE.  Also sets the
-   non-permission bits of the destination file (S_ISUID, S_ISGID, S_ISVTX)
-   to those from MODE if any are set.
+/* Set the access control lists of a file to match EXACTLY mode (this might
+   remove inherited ACLs). Note chmod() tends to honor inherited/default
+   ACLs. If DESC is a valid file descriptor, use file descriptor operations 
+   where available, else use filename based operations on NAME.  If access 
+   control lists are not available, fchmod the target file to MODE.  Also 
+   sets the non-permission bits of the destination file 
+   (S_ISUID, S_ISGID, S_ISVTX) to those from MODE if any are set.
    Return 0 if successful.  On failure, output a diagnostic, set errno and
    return -1.  */
 
-- 
2.39.0

Re: [PATCH] better explain functions [q]set-acl.c

[Bruno Haible]

Ondrej Valousek wrote:
> +/* Set the access control lists of a file to match EXACTLY mode (this might

In function comments, by convention we refer to parameters by uppercasing their
name. So, this line should probably better read

   /* Set the access control lists of a file to match *exactly* MODE (this might

Bruno

[PATCH] better explain functions [q]set-acl.c

[Ondrej Valousek]

Improve comments for both functions

---
 lib/qset-acl.c | 16 +++++++++-------
 lib/set-acl.c  | 13 +++++++------
 2 files changed, 16 insertions(+), 13 deletions(-)

diff --git a/lib/qset-acl.c b/lib/qset-acl.c
index c3442d060f..0ae026f031 100644
--- a/lib/qset-acl.c
+++ b/lib/qset-acl.c
@@ -27,13 +27,15 @@
 #include "acl-internal.h"
 
 
-/* Set the access control lists of a file. If DESC is a valid file
-   descriptor, use file descriptor operations where available, else use
-   filename based operations on NAME.  If access control lists are not
-   available, fchmod the target file to MODE.  Also sets the
-   non-permission bits of the destination file (S_ISUID, S_ISGID, S_ISVTX)
-   to those from MODE if any are set.
+/* Set the access control lists of a file to match EXACTLY mode (this might
+   remove inherited ACLs). Note chmod() tends to honor inherited/default
+   ACLs. If DESC is a valid file descriptor, use file descriptor operations 
+   where available, else use filename based operations on NAME.  If access 
+   control lists are not available, fchmod the target file to MODE.  Also 
+   sets the non-permission bits of the destination file 
+   (S_ISUID, S_ISGID, S_ISVTX) to those from MODE if any are set.
    Return 0 if successful.  Return -1 and set errno upon failure.  */
 
 int
 qset_acl (char const *name, int desc, mode_t mode)
diff --git a/lib/set-acl.c b/lib/set-acl.c
index 9111911864..561ff47824 100644
--- a/lib/set-acl.c
+++ b/lib/set-acl.c
@@ -29,12 +29,13 @@
 #define _(msgid) gettext (msgid)
 
 
-/* Set the access control lists of a file. If DESC is a valid file
-   descriptor, use file descriptor operations where available, else use
-   filename based operations on NAME.  If access control lists are not
-   available, fchmod the target file to MODE.  Also sets the
-   non-permission bits of the destination file (S_ISUID, S_ISGID, S_ISVTX)
-   to those from MODE if any are set.
+/* Set the access control lists of a file to match EXACTLY mode (this might
+   remove inherited ACLs). Note chmod() tends to honor inherited/default
+   ACLs. If DESC is a valid file descriptor, use file descriptor operations 
+   where available, else use filename based operations on NAME.  If access 
+   control lists are not available, fchmod the target file to MODE.  Also 
+   sets the non-permission bits of the destination file 
+   (S_ISUID, S_ISGID, S_ISVTX) to those from MODE if any are set.
    Return 0 if successful.  On failure, output a diagnostic, set errno and
    return -1.  */
 
-- 
2.39.0

Re: [PATCH] better explain functions [q]set-acl.c

[Bruno Haible]

Hi Ondrej,

> Improve comments for both functions

Did you see the feedback that I gave in
https://lists.gnu.org/archive/html/bug-gnulib/2023-01/msg00063.html ?

Bruno

[PATCH] better explain functions [q]set-acl.c

[Ondrej Valousek]

Improve comments for both functions

---
 lib/qset-acl.c | 16 +++++++++-------
 lib/set-acl.c  | 13 +++++++------
 2 files changed, 16 insertions(+), 13 deletions(-)

diff --git a/lib/qset-acl.c b/lib/qset-acl.c
index c3442d060f..0ae026f031 100644
--- a/lib/qset-acl.c
+++ b/lib/qset-acl.c
@@ -27,13 +27,15 @@
 #include "acl-internal.h"
 
 
-/* Set the access control lists of a file. If DESC is a valid file
-   descriptor, use file descriptor operations where available, else use
-   filename based operations on NAME.  If access control lists are not
-   available, fchmod the target file to MODE.  Also sets the
-   non-permission bits of the destination file (S_ISUID, S_ISGID, S_ISVTX)
-   to those from MODE if any are set.
+/* Set the access control lists of a file to match *exactly* MODE (this might
+   remove inherited ACLs). Note chmod() tends to honor inherited/default
+   ACLs. If DESC is a valid file descriptor, use file descriptor operations 
+   where available, else use filename based operations on NAME.  If access 
+   control lists are not available, fchmod the target file to MODE.  Also 
+   sets the non-permission bits of the destination file 
+   (S_ISUID, S_ISGID, S_ISVTX) to those from MODE if any are set.
    Return 0 if successful.  Return -1 and set errno upon failure.  */
 
 int
 qset_acl (char const *name, int desc, mode_t mode)
diff --git a/lib/set-acl.c b/lib/set-acl.c
index 9111911864..561ff47824 100644
--- a/lib/set-acl.c
+++ b/lib/set-acl.c
@@ -29,12 +29,13 @@
 #define _(msgid) gettext (msgid)
 
 
-/* Set the access control lists of a file. If DESC is a valid file
-   descriptor, use file descriptor operations where available, else use
-   filename based operations on NAME.  If access control lists are not
-   available, fchmod the target file to MODE.  Also sets the
-   non-permission bits of the destination file (S_ISUID, S_ISGID, S_ISVTX)
-   to those from MODE if any are set.
+/* Set the access control lists of a file to match *exactly* MODE (this might
+   remove inherited ACLs). Note chmod() tends to honor inherited/default
+   ACLs. If DESC is a valid file descriptor, use file descriptor operations 
+   where available, else use filename based operations on NAME.  If access 
+   control lists are not available, fchmod the target file to MODE.  Also 
+   sets the non-permission bits of the destination file 
+   (S_ISUID, S_ISGID, S_ISVTX) to those from MODE if any are set.
    Return 0 if successful.  On failure, output a diagnostic, set errno and
    return -1.  */
 
-- 
2.39.0

Re: [PATCH] better explain functions [q]set-acl.c

[Bruno Haible]

Hi Ondrej,

> Improve comments for both functions
> 
> ---
>  lib/qset-acl.c | 16 +++++++++-------
>  lib/set-acl.c  | 13 +++++++------
>  2 files changed, 16 insertions(+), 13 deletions(-)

I wanted to apply this patch in your name, but it fails:
error: corrupt patch at line 31

Evidently the patch is not right, because its first hunk has 4 unmodified
lines at the end, instead of 3.

Please
1. make sure that the patch does not add trailing whitespace ("git diff"
   shows red spaces when this happens),
2. regenerate the patch using 'git format-patch', so that each hunk has 3
   unmodified lines at the beginning and at the end,
3. send the patch as an attachment. Your mailer apparently modifies URLs
   in text that you send; we don't need this.

Bruno