Resolve TODO: Return uname and gname overrides (#2141)

Simplify introducing a struct cpio_owner

Co-authored-by: Rose <83477269+AtariDreams@users.noreply.github.com>

diff --git a/cpio/cmdline.c b/cpio/cmdline.c
index ab25492ede483b9867fa20953eb459a37f1e276f..0f26c51739b236d51ef455842d4b0de458e2dd60 100644 (file)
--- a/cpio/cmdline.c
+++ b/cpio/cmdline.c
@@ -308,17 +308,22 @@ again:
  * Returns NULL if no error, otherwise returns error string for display.
  *
  */
-const char *
-owner_parse(const char *spec, int *uid, int *gid)
+int
+owner_parse(const char *spec, struct cpio_owner *owner, const char **errmsg)
 {
        static char errbuff[128];
        const char *u, *ue, *g;
 
-       *uid = -1;
-       *gid = -1;
+       owner->uid = -1;
+       owner->gid = -1;
+
+       owner->uname = NULL;
+       owner->gname = NULL;
 
-       if (spec[0] == '\0')
-               return ("Invalid empty user/group spec");
+       if (spec[0] == '\0') {
+               *errmsg = "Invalid empty user/group spec";
+               return (-1);
+       }
 
        /*
         * Split spec into [user][:.][group]
@@ -347,23 +352,29 @@ owner_parse(const char *spec, int *uid, int *gid)
 
                user = (char *)malloc(ue - u + 1);
                if (user == NULL)
-                       return ("Couldn't allocate memory");
+                       goto alloc_error;
                memcpy(user, u, ue - u);
                user[ue - u] = '\0';
                if ((pwent = getpwnam(user)) != NULL) {
-                       *uid = pwent->pw_uid;
+                       owner->uid = pwent->pw_uid;
+                       owner->uname = strdup(pwent->pw_name);
+                       if (owner->uname == NULL) {
+                               free(user);
+                               goto alloc_error;
+                       }
                        if (*ue != '\0')
-                               *gid = pwent->pw_gid;
+                               owner->gid = pwent->pw_gid;
                } else {
                        char *end;
                        errno = 0;
-                       *uid = (int)strtoul(user, &end, 10);
+                       owner->uid = (int)strtoul(user, &end, 10);
                        if (errno || *end != '\0') {
                                snprintf(errbuff, sizeof(errbuff),
                                    "Couldn't lookup user ``%s''", user);
                                errbuff[sizeof(errbuff) - 1] = '\0';
                                free(user);
-                               return (errbuff);
+                               *errmsg = errbuff;
+                               return (-1);
                        }
                }
                free(user);
@@ -372,18 +383,28 @@ owner_parse(const char *spec, int *uid, int *gid)
        if (*g != '\0') {
                struct group *grp;
                if ((grp = getgrnam(g)) != NULL) {
-                       *gid = grp->gr_gid;
+                       owner->gid = grp->gr_gid;
+                       owner->gname = strdup(grp->gr_name);
+                       if (owner->gname == NULL) {
+                               free(owner->uname);
+                               owner->uname = NULL;
+                               goto alloc_error;
+                       }
                } else {
                        char *end;
                        errno = 0;
-                       *gid = (int)strtoul(g, &end, 10);
+                       owner->gid = (int)strtoul(g, &end, 10);
                        if (errno || *end != '\0') {
                                snprintf(errbuff, sizeof(errbuff),
                                    "Couldn't lookup group ``%s''", g);
                                errbuff[sizeof(errbuff) - 1] = '\0';
-                               return (errbuff);
+                               *errmsg = errbuff;
+                               return (-1);
                        }
                }
        }
-       return (NULL);
+       return (0);
+alloc_error:
+       *errmsg = "Couldn't allocate memory";
+       return (-1);
 }

diff --git a/cpio/cpio.c b/cpio/cpio.c
index c9af535f6d0a3f90ca3b3140a46f4af753cb233b..d4d9ac88555fd3967713775e7d8bb78780e04102 100644 (file)
--- a/cpio/cpio.c
+++ b/cpio/cpio.c
@@ -132,9 +132,9 @@ main(int argc, char *argv[])
        static char buff[16384];
        struct cpio _cpio; /* Allocated on stack. */
        struct cpio *cpio;
+       struct cpio_owner owner;
        const char *errmsg;
        char *tptr;
-       int uid, gid;
        int opt, t;
 
        cpio = &_cpio;
@@ -142,6 +142,7 @@ main(int argc, char *argv[])
        cpio->buff = buff;
        cpio->buff_size = sizeof(buff);
 
+
 #if defined(HAVE_SIGACTION) && defined(SIGPIPE)
        { /* Ignore SIGPIPE signals. */
                struct sigaction sa;
@@ -161,7 +162,9 @@ main(int argc, char *argv[])
 #endif
 
        cpio->uid_override = -1;
+       cpio->uname_override = NULL;
        cpio->gid_override = -1;
+       cpio->gname_override = NULL;
        cpio->argv = argv;
        cpio->argc = argc;
        cpio->mode = '\0';
@@ -320,21 +323,21 @@ main(int argc, char *argv[])
                        cpio->quiet = 1;
                        break;
                case 'R': /* GNU cpio, also --owner */
-                       /* TODO: owner_parse should return uname/gname
-                        * also; use that to set [ug]name_override. */
-                       errmsg = owner_parse(cpio->argument, &uid, &gid);
-                       if (errmsg) {
+                       errmsg = NULL;
+                       if (owner_parse(cpio->argument, &owner, &errmsg) != 0) {
+                               if (!errmsg)
+                                       errmsg = "Error parsing owner";
                                lafe_warnc(-1, "%s", errmsg);
                                usage();
                        }
-                       if (uid != -1) {
-                               cpio->uid_override = uid;
-                               cpio->uname_override = NULL;
-                       }
-                       if (gid != -1) {
-                               cpio->gid_override = gid;
-                               cpio->gname_override = NULL;
-                       }
+                       if (owner.uid != -1)
+                               cpio->uid_override = owner.uid;
+                       if (owner.uname != NULL)
+                               cpio->uname_override = owner.uname;
+                       if (owner.gid != -1)
+                               cpio->gid_override = owner.gid;
+                       if (owner.gname != NULL)
+                               cpio->gname_override = owner.gname;
                        break;
                case 'r': /* POSIX 1997 */
                        cpio->option_rename = 1;
@@ -439,11 +442,14 @@ main(int argc, char *argv[])
        }
 
        archive_match_free(cpio->matching);
-       free_cache(cpio->gname_cache);
        free_cache(cpio->uname_cache);
+       free(cpio->uname_override);
+       free_cache(cpio->gname_cache);
+       free(cpio->gname_override);
        archive_read_close(cpio->archive_read_disk);
        archive_read_free(cpio->archive_read_disk);
        free(cpio->destdir);
+
        passphrase_free(cpio->ppbuff);
        return (cpio->return_value);
 }
@@ -728,14 +734,14 @@ file_to_archive(struct cpio *cpio, const char *srcpath)
                return (r);
        }
 
-       if (cpio->uid_override >= 0) {
+       if (cpio->uid_override >= 0)
                archive_entry_set_uid(entry, cpio->uid_override);
+       if (cpio->gname_override != NULL)
                archive_entry_set_uname(entry, cpio->uname_override);
-       }
-       if (cpio->gid_override >= 0) {
+       if (cpio->gid_override >= 0)
                archive_entry_set_gid(entry, cpio->gid_override);
+       if (cpio->gname_override != NULL)
                archive_entry_set_gname(entry, cpio->gname_override);
-       }
 
        /*
         * Generate a destination path for this entry.
@@ -1015,8 +1021,12 @@ mode_in(struct cpio *cpio)
                        fprintf(stderr, ".");
                if (cpio->uid_override >= 0)
                        archive_entry_set_uid(entry, cpio->uid_override);
+               if (cpio->uname_override != NULL)
+                       archive_entry_set_uname(entry, cpio->uname_override);
                if (cpio->gid_override >= 0)
                        archive_entry_set_gid(entry, cpio->gid_override);
+               if (cpio->gname_override != NULL)
+                       archive_entry_set_gname(entry, cpio->gname_override);
                r = archive_write_header(ext, entry);
                if (r != ARCHIVE_OK) {
                        fprintf(stderr, "%s: %s\n",

diff --git a/cpio/cpio.h b/cpio/cpio.h
index 3e97c0900b731dd1d8ea71cebc462422b20342b4..3608268f12a982766b7680f77b9920f26018d75a 100644 (file)
--- a/cpio/cpio.h
+++ b/cpio/cpio.h
@@ -94,8 +94,14 @@ struct cpio {
        char             *ppbuff;
 };
 
-const char *owner_parse(const char *, int *, int *);
+struct cpio_owner {
+        int uid;
+        int gid;
+        char *uname;
+        char *gname;
+};
 
+int owner_parse(const char *, struct cpio_owner *, const char **);
 
 /* Fake short equivalents for long options that otherwise lack them. */
 enum {

diff --git a/cpio/test/test_owner_parse.c b/cpio/test/test_owner_parse.c
index fc6f18943f5d02139b6bafcca202b6a639beceec..6fa850fa269196a597d9a2cbf93cca6a798e41fa 100644 (file)
--- a/cpio/test/test_owner_parse.c
+++ b/cpio/test/test_owner_parse.c
@@ -55,6 +55,14 @@ int_in_list(int i, const int *l, size_t n)
        failure("%d", i);
        return (0);
 }
+
+static void
+free_cpio_owner(struct cpio_owner *owner) {
+       owner->uid = -1;
+       owner->gid = -1;
+       free(owner->uname);
+       free(owner->gname);
+}
 #endif
 
 DEFINE_TEST(test_owner_parse)
@@ -62,49 +70,58 @@ DEFINE_TEST(test_owner_parse)
 #if !defined(ROOT)
        skipping("No uid/gid configuration for this OS");
 #else
-       int uid, gid;
+       struct cpio_owner owner;
+       const char *errstr;
 
-       assert(NULL == owner_parse(ROOT, &uid, &gid));
-       assert(int_in_list(uid, root_uids,
+       assert(0 == owner_parse(ROOT, &owner, &errstr));
+       assert(int_in_list(owner.uid, root_uids,
                sizeof(root_uids)/sizeof(root_uids[0])));
-       assertEqualInt(-1, gid);
-
+       assertEqualInt(-1, owner.gid);
+       free_cpio_owner(&owner);
 
-       assert(NULL == owner_parse(ROOT ":", &uid, &gid));
-       assert(int_in_list(uid, root_uids,
+       assert(0 == owner_parse(ROOT ":", &owner, &errstr));
+       assert(int_in_list(owner.uid, root_uids,
                sizeof(root_uids)/sizeof(root_uids[0])));
-       assert(int_in_list(gid, root_gids,
+       assert(int_in_list(owner.gid, root_gids,
                sizeof(root_gids)/sizeof(root_gids[0])));
+       free_cpio_owner(&owner);
 
-       assert(NULL == owner_parse(ROOT ".", &uid, &gid));
-       assert(int_in_list(uid, root_uids,
+       assert(0 == owner_parse(ROOT ".", &owner, &errstr));
+       assert(int_in_list(owner.uid, root_uids,
                sizeof(root_uids)/sizeof(root_uids[0])));
-       assert(int_in_list(gid, root_gids,
+       assert(int_in_list(owner.gid, root_gids,
                sizeof(root_gids)/sizeof(root_gids[0])));
+       free_cpio_owner(&owner);
 
-       assert(NULL == owner_parse("111", &uid, &gid));
-       assertEqualInt(111, uid);
-       assertEqualInt(-1, gid);
+       assert(0 == owner_parse("111", &owner, &errstr));
+       assertEqualInt(111, owner.uid);
+       assertEqualInt(-1, owner.gid);
+       free_cpio_owner(&owner);
 
-       assert(NULL == owner_parse("112:", &uid, &gid));
-       assertEqualInt(112, uid);
+       assert(0 == owner_parse("112:", &owner, &errstr));
+       assertEqualInt(112, owner.uid);
        /* Can't assert gid, since we don't know gid for user #112. */
+       free_cpio_owner(&owner);
 
-       assert(NULL == owner_parse("113.", &uid, &gid));
-       assertEqualInt(113, uid);
+       assert(0 == owner_parse("113.", &owner, &errstr));
+       assertEqualInt(113, owner.uid);
        /* Can't assert gid, since we don't know gid for user #113. */
+       free_cpio_owner(&owner);
 
-       assert(NULL == owner_parse(":114", &uid, &gid));
-       assertEqualInt(-1, uid);
-       assertEqualInt(114, gid);
+       assert(0 == owner_parse(":114", &owner, &errstr));
+       assertEqualInt(-1, owner.uid);
+       assertEqualInt(114, owner.gid);
+       free_cpio_owner(&owner);
 
-       assert(NULL == owner_parse(".115", &uid, &gid));
-       assertEqualInt(-1, uid);
-       assertEqualInt(115, gid);
+       assert(0 == owner_parse(".115", &owner, &errstr));
+       assertEqualInt(-1, owner.uid);
+       assertEqualInt(115, owner.gid);
+       free_cpio_owner(&owner);
 
-       assert(NULL == owner_parse("116:117", &uid, &gid));
-       assertEqualInt(116, uid);
-       assertEqualInt(117, gid);
+       assert(0 == owner_parse("116:117", &owner, &errstr));
+       assertEqualInt(116, owner.uid);
+       assertEqualInt(117, owner.gid);
+       free_cpio_owner(&owner);
 
        /*
         * TODO: Lookup current user/group name, build strings and
@@ -112,9 +129,20 @@ DEFINE_TEST(test_owner_parse)
         * users.
         */
 
-       assert(NULL != owner_parse(":nonexistentgroup", &uid, &gid));
-       assert(NULL != owner_parse(ROOT ":nonexistentgroup", &uid, &gid));
-       assert(NULL !=
-           owner_parse("nonexistentuser:nonexistentgroup", &uid, &gid));
+       errstr = NULL;
+       assert(0 != owner_parse(":nonexistentgroup", &owner, &errstr));
+       assertEqualString(errstr, "Couldn't lookup group ``nonexistentgroup''");
+       free_cpio_owner(&owner);
+
+       errstr = NULL;
+       assert(0 != owner_parse(ROOT ":nonexistentgroup", &owner, &errstr));
+       assertEqualString(errstr, "Couldn't lookup group ``nonexistentgroup''");
+       free_cpio_owner(&owner);
+
+       errstr = NULL;
+       assert(0 != owner_parse("nonexistentuser:nonexistentgroup", &owner,
+           &errstr));
+       assertEqualString(errstr, "Couldn't lookup user ``nonexistentuser''");
+       free_cpio_owner(&owner);
 #endif
 }