acl-devel
[Top][All Lists]
Advanced

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

[PATCH 1/3] Add advanced error reporting


From: Bodo Eggert
Subject: [PATCH 1/3] Add advanced error reporting
Date: Tue, 26 Sep 2023 17:09:22 +0200

It's a bad thing to report error while retrieving the user / group name as
"Invalid argument". To change that a more specific error message needs to be
passed up to the calling function.

This first version does add this feature, the error message would usually be
"" unless a more specific error can reasonably be displayed; then it will e.g.
print " (User not found)" in addition to the usual error message.
---
 tools/parse.c   | 78 ++++++++++++++++++++++++++++++++++++++++---------
 tools/parse.h   | 11 +++++--
 tools/setfacl.c | 25 ++++++++--------
 3 files changed, 86 insertions(+), 28 deletions(-)

diff --git a/tools/parse.c b/tools/parse.c
index 78ae49a..b4d0b7d 100644
--- a/tools/parse.c
+++ b/tools/parse.c
@@ -42,6 +42,25 @@
                (x)++; \
        })
 
+/*char *advanced_errors[] = {
+       "OK",
+       "(generic)",
+       "default acl exists",
+       "User not found",
+       "Error while looking up user",
+       "Group not found",
+       "Error while looking up group",
+};*/
+char *advanced_errors[] = {
+       "",
+       "",
+       " (default acl exists)",
+       " (User not found)",
+       " (Error while looking up user)",
+       " (Group not found)",
+       " (Error while looking up group)",
+};
+
 
 static int
 skip_tag_name(
@@ -142,12 +161,13 @@ get_uid(
 
        if (get_id(token, (id_t *)uid_p) == 0)
                goto accept;
+       errno = 0;
        passwd = getpwnam(token);
        if (passwd) {
                *uid_p = passwd->pw_uid;
                goto accept;
        }
-       return -1;
+       return errno? 2 : 1;
 
 accept:
        return 0;
@@ -163,12 +183,13 @@ get_gid(
 
        if (get_id(token, (id_t *)gid_p) == 0)
                goto accept;
+       errno = 0;
        group = getgrnam(token);
        if (group) {
                *gid_p = group->gr_gid;
                goto accept;
        }
-       return -1;
+       return errno? 2 : 1;
 
 accept:
        return 0;
@@ -179,19 +200,30 @@ accept:
        Parses the next acl entry in text_p.
 
        Returns:
-               -1 on error, 0 on success.
+               cmd_t on success
+               NULL on failure
+               &err_ret ==
+                0: OK
+                1: (generic)
+                2: default acl exists
+                3: User not found
+                4: Error while looking up user
+                5: Group not found
+                6: Error while looking up group
 */
 
 cmd_t
 parse_acl_cmd(
        const char **text_p,
        int seq_cmd,
-       int parse_mode)
+       int parse_mode,
+       int *err_ret)
 {
        cmd_t cmd = cmd_init();
        char *str;
        const char *backup;
        int error, perm_chars;
+       *err_ret = 0;
        if (!cmd)
                return NULL;
 
@@ -211,6 +243,7 @@ parse_acl_cmd(
                                /* if promoting from acl to default acl and
                                   a default acl entry is found, fail. */
                                *text_p = backup;
+                               *err_ret = 2;
                                goto fail;
                        }
                        cmd->c_type = ACL_TYPE_DEFAULT;
@@ -231,6 +264,7 @@ user_entry:
                                free(str);
                                if (error) {
                                        *text_p = backup;
+                                       *err_ret = 2 + error;
                                        goto fail;
                                }
                        } else {
@@ -250,6 +284,7 @@ user_entry:
                                free(str);
                                if (error) {
                                        *text_p = backup;
+                                       *err_ret = 4 + error;
                                        goto fail;
                                }
                        } else {
@@ -288,6 +323,7 @@ user_entry:
                if (parse_mode & SEQ_PARSE_NO_PERM)
                        return cmd;
                else
+                       *err_ret = 1;
                        goto fail;
        }
        if (!(parse_mode & SEQ_PARSE_WITH_PERM))
@@ -309,28 +345,36 @@ user_entry:
        for (perm_chars=0;; perm_chars++, (*text_p)++) {
                switch(**text_p) {
                        case 'r': /* read */
-                               if (cmd->c_perm & CMD_PERM_READ)
+                               if (cmd->c_perm & CMD_PERM_READ) {
+                                       *err_ret = 1;
                                        goto fail;
+                               }
                                cmd->c_perm |= CMD_PERM_READ;
                                break;
 
                        case 'w':  /* write */
-                               if (cmd->c_perm & CMD_PERM_WRITE)
+                               if (cmd->c_perm & CMD_PERM_WRITE) {
+                                       *err_ret = 1;
                                        goto fail;
+                               }
                                cmd->c_perm |= CMD_PERM_WRITE;
                                break;
 
                        case 'x':  /* execute */
-                               if (cmd->c_perm & CMD_PERM_EXECUTE)
+                               if (cmd->c_perm & CMD_PERM_EXECUTE) {
+                                       *err_ret = 1;
                                        goto fail;
+                               }
                                cmd->c_perm |= CMD_PERM_EXECUTE;
                                break;
 
                        case 'X':  /* execute only if directory or some
                                      entries already have execute permissions
                                      set */
-                               if (cmd->c_perm & CMD_PERM_COND_EXECUTE)
+                               if (cmd->c_perm & CMD_PERM_COND_EXECUTE) {
+                                       *err_ret = 1;
                                        goto fail;
+                               }
                                cmd->c_perm |= CMD_PERM_COND_EXECUTE;
                                break;
 
@@ -339,8 +383,10 @@ user_entry:
                                break;
 
                        default:
-                               if (perm_chars == 0)
+                               if (perm_chars == 0) {
+                                       *err_ret = 1;
                                        goto fail;
+                               }
                                return cmd;
                }
        }
@@ -364,16 +410,19 @@ parse_acl_seq(
        const char *text_p,
        int *which,
        int seq_cmd,
-       int parse_mode)
+       int parse_mode,
+       int *err_ret)
 {
        const char *initial_text_p = text_p;
        cmd_t cmd;
 
+       *err_ret = 0;
+
        if (which)
                *which = -1;
 
        while (*text_p != '\0') {
-               cmd = parse_acl_cmd(&text_p, seq_cmd, parse_mode);
+               cmd = parse_acl_cmd(&text_p, seq_cmd, parse_mode, err_ret);
                if (cmd == NULL) {
                        errno = EINVAL;
                        goto fail;
@@ -533,12 +582,15 @@ read_acl_seq(
        int seq_cmd,
        int parse_mode,
        int *lineno,
-       int *which)
+       int *which,
+       int *err_ret)
 {
        char *line;
        const char *cp;
        cmd_t cmd;
 
+       *err_ret = 0;
+
        if (which)
                *which = -1;
 
@@ -556,7 +608,7 @@ read_acl_seq(
                        continue;
                }
 
-               cmd = parse_acl_cmd(&cp, seq_cmd, parse_mode);
+               cmd = parse_acl_cmd(&cp, seq_cmd, parse_mode, err_ret);
                if (cmd == NULL) {
                        errno = EINVAL;
                        goto fail;
diff --git a/tools/parse.h b/tools/parse.h
index 2549753..50d2342 100644
--- a/tools/parse.h
+++ b/tools/parse.h
@@ -46,18 +46,22 @@ extern "C" {
 #define SEQ_PROMOTE_ACL                (0x0040)        /* promote from acl
                                                    to default acl */
 
+extern char *advanced_errors[];
+
 cmd_t
 parse_acl_cmd(
        const char **text_p,
        int seq_cmd,
-       int parse_mode);
+       int parse_mode,
+       int *err_ret);
 int
 parse_acl_seq(
        seq_t seq,
        const char *text_p,
        int *which,
        int seq_cmd,
-       int parse_mode);
+       int parse_mode,
+       int *err_ret);
 int
 read_acl_comments(
        FILE *file,
@@ -73,7 +77,8 @@ read_acl_seq(
        int seq_cmd,
        int parse_mode,
        int *lineno,
-       int *which);
+       int *which,
+       int *err_ret);
 
 
 #ifdef __cplusplus
diff --git a/tools/setfacl.c b/tools/setfacl.c
index fd0bf2e..fe2f79d 100644
--- a/tools/setfacl.c
+++ b/tools/setfacl.c
@@ -128,6 +128,7 @@ restore(
        int lineno = 0, backup_line;
        int error, status = 0;
        int chmod_required = 0;
+       int err_ret;
 
        memset(&st, 0, sizeof(st));
 
@@ -168,11 +169,11 @@ restore(
                                     SEQ_PARSE_WITH_PERM |
                                     SEQ_PARSE_DEFAULT |
                                     SEQ_PARSE_MULTI,
-                                    &lineno, NULL);
+                                    &lineno, NULL, &err_ret);
                if (error != 0) {
-                       fprintf(stderr, _("%s: %s: %s in line %d\n"),
+                       fprintf(stderr, _("%s: %s: %s%s in line %d\n"),
                                progname, xquote(filename, "\n\r"), 
strerror(errno),
-                               lineno);
+                               advanced_errors[err_ret], lineno);
                        status = 1;
                        goto getout;
                }
@@ -338,7 +339,7 @@ int main(int argc, char *argv[])
        int lineno;
        int error;
        seq_t seq;
-       int seq_cmd, parse_mode;
+       int seq_cmd, parse_mode, err_ret;
        
        progname = basename(argv[0]);
 
@@ -445,7 +446,7 @@ int main(int argc, char *argv[])
                                if (opt_promote)
                                        parse_mode |= SEQ_PROMOTE_ACL;
                                if (parse_acl_seq(seq, optarg, &which,
-                                                 seq_cmd, parse_mode) != 0) {
+                                                 seq_cmd, parse_mode, 
&err_ret) != 0) {
                                        if (which < 0 ||
                                            (size_t) which >= strlen(optarg)) {
                                                fprintf(stderr, _(
@@ -455,10 +456,10 @@ int main(int argc, char *argv[])
                                        } else {
                                                fprintf(stderr, _(
                                                        "%s: Option "
-                                                       "-%c: %s near "
+                                                       "-%c: %s%s near "
                                                        "character %d\n"),
                                                        progname, opt,
-                                                       strerror(errno),
+                                                       strerror(errno), 
advanced_errors[err_ret],
                                                        which+1);
                                        }
                                        status = 2;
@@ -518,7 +519,7 @@ int main(int argc, char *argv[])
 
                                lineno = 0;
                                error = read_acl_seq(file, seq, seq_cmd,
-                                                    parse_mode, &lineno, NULL);
+                                                    parse_mode, &lineno, NULL, 
&err_ret);
                                
                                if (file != stdin) {
                                        fclose(file);
@@ -530,18 +531,18 @@ int main(int argc, char *argv[])
 
                                        if (file != stdin) {
                                                fprintf(stderr, _(
-                                                       "%s: %s in line "
+                                                       "%s: %s%s in line "
                                                        "%d of file %s\n"),
                                                        progname,
-                                                       strerror(errno),
+                                                       strerror(errno), 
advanced_errors[err_ret],
                                                        lineno,
                                                        xquote(optarg, "\n\r"));
                                        } else {
                                                fprintf(stderr, _(
-                                                       "%s: %s in line "
+                                                       "%s: %s%s in line "
                                                        "%d of standard "
                                                        "input\n"), progname,
-                                                       strerror(errno),
+                                                       strerror(errno), 
advanced_errors[err_ret],
                                                        lineno);
                                        }
                                        status = 2;
-- 
2.42.0




reply via email to

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