Date: Mon, 6 Jul 2009 02:02:45 +0000 (UTC) From: Tim Kientzle <kientzle@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r195389 - in head/usr.bin/cpio: . test Message-ID: <200907060202.n6622jw9065554@svn.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: kientzle Date: Mon Jul 6 02:02:45 2009 New Revision: 195389 URL: http://svn.freebsd.org/changeset/base/195389 Log: This addresses some issues with my earlier -R fix that were pointed out by Brooks Davis and Alexey Dokuchaev: * It now tries to lookup arguments as names first, then tries to parse them as numbers. In particular, this makes the behavior consistent with POSIX conventions when usernames consist entirely of digits. * It now uses strtoul() for the numeric parsing. Finally, I've included an update to the test harness to exercise the new numeric cases for -R. Approved by: re (kib) Modified: head/usr.bin/cpio/cmdline.c head/usr.bin/cpio/test/test_owner_parse.c Modified: head/usr.bin/cpio/cmdline.c ============================================================================== --- head/usr.bin/cpio/cmdline.c Mon Jul 6 01:32:29 2009 (r195388) +++ head/usr.bin/cpio/cmdline.c Mon Jul 6 02:02:45 2009 (r195389) @@ -275,29 +275,14 @@ cpio_getopt(struct cpio *cpio) * :<groupname|gid> - Override group but not user * * Where uid/gid are decimal representations and groupname/username - * are names to be looked up in system database. Note that - * uid/gid parsing takes priority over username/groupname lookup, - * so this won't do a lookup for usernames or group names that - * consist entirely of digits. + * are names to be looked up in system database. Note that we try + * to look up an argument as a name first, then try numeric parsing. * * A period can be used instead of the colon. * * Sets uid/gid return as appropriate, -1 indicates uid/gid not specified. * */ -static int -decimal_parse(const char *p) -{ - /* TODO: guard against overflow. */ - int n = 0; - for (; *p != '\0'; ++p) { - if (*p < '0' || *p > '9') - return (-1); - n = n * 10 + *p - '0'; - } - return (n); -} - int owner_parse(const char *spec, int *uid, int *gid) { @@ -306,6 +291,9 @@ owner_parse(const char *spec, int *uid, *uid = -1; *gid = -1; + if (spec[0] == '\0') + return (1); + /* * Split spec into [user][:.][group] * u -> first char of username, NULL if no username @@ -338,32 +326,34 @@ owner_parse(const char *spec, int *uid, } memcpy(user, u, ue - u); user[ue - u] = '\0'; - *uid = decimal_parse(user); - if (*uid < 0) { - /* Couldn't parse as integer, try username lookup. */ - pwent = getpwnam(user); - if (pwent == NULL) { + if ((pwent = getpwnam(user)) != NULL) { + *uid = pwent->pw_uid; + if (*ue != '\0') + *gid = pwent->pw_gid; + } else { + char *end; + errno = 0; + *uid = strtoul(user, &end, 10); + if (errno || *end != '\0') { cpio_warnc(errno, "Couldn't lookup user ``%s''", user); return (1); } - *uid = pwent->pw_uid; - if (*ue != '\0' && *g == '\0') - *gid = pwent->pw_gid; } free(user); } + if (*g != '\0') { - *gid = decimal_parse(g); - if (*gid < 0) { - /* Couldn't parse int, try group name lookup. */ - struct group *grp; - grp = getgrnam(g); - if (grp != NULL) - *gid = grp->gr_gid; - else { + struct group *grp; + if ((grp = getgrnam(g)) != NULL) { + *gid = grp->gr_gid; + } else { + char *end; + errno = 0; + *gid = strtoul(g, &end, 10); + if (errno || *end != '\0') { cpio_warnc(errno, - "Couldn't look up group ``%s''", g); + "Couldn't lookup group ``%s''", g); return (1); } } Modified: head/usr.bin/cpio/test/test_owner_parse.c ============================================================================== --- head/usr.bin/cpio/test/test_owner_parse.c Mon Jul 6 01:32:29 2009 (r195388) +++ head/usr.bin/cpio/test/test_owner_parse.c Mon Jul 6 02:02:45 2009 (r195389) @@ -1,5 +1,5 @@ /*- - * Copyright (c) 2003-2007 Tim Kientzle + * Copyright (c) 2003-2009 Tim Kientzle * All rights reserved. * * Redistribution and use in source and binary forms, with or without @@ -26,6 +26,7 @@ __FBSDID("$FreeBSD$"); #include "../cpio.h" +#include "err.h" #if defined(__CYGWIN__) /* On cygwin, the Administrator user most likely exists (unless @@ -37,16 +38,23 @@ __FBSDID("$FreeBSD$"); * Use CreateWellKnownSID() and LookupAccountName()? */ #define ROOT "Administrator" -#define ROOT_UID 500 -#define ROOT_GID1 513 -#define ROOT_GID2 545 -#define ROOT_GID3 544 +static int root_uids[] = { 500 }; +static int root_gids[] = { 513, 545, 544 }; #else #define ROOT "root" -#define ROOT_UID 0 -#define ROOT_GID 0 +static int root_uids[] = { 0 }; +static int root_gids[] = { 0 }; #endif +static int +int_in_list(int i, int *l, size_t n) +{ + while (n-- > 0) + if (*l++ == i) + return (1); + failure("%d", i); + return (0); +} DEFINE_TEST(test_owner_parse) { @@ -56,38 +64,47 @@ DEFINE_TEST(test_owner_parse) #else int uid, gid; - cpio_progname = "Ignore this message"; - assertEqualInt(0, owner_parse(ROOT, &uid, &gid)); - assertEqualInt(ROOT_UID, uid); + assert(int_in_list(uid, root_uids, + sizeof(root_uids)/sizeof(root_uids[0]))); assertEqualInt(-1, gid); assertEqualInt(0, owner_parse(ROOT ":", &uid, &gid)); - assertEqualInt(ROOT_UID, uid); -#if defined(__CYGWIN__) - { - int gidIsOneOf = (ROOT_GID1 == gid) - || (ROOT_GID2 == gid) - || (ROOT_GID3 == gid); - assertEqualInt(1, gidIsOneOf); - } -#else - assertEqualInt(ROOT_GID, gid); -#endif + assert(int_in_list(uid, root_uids, + sizeof(root_uids)/sizeof(root_uids[0]))); + assert(int_in_list(gid, root_gids, + sizeof(root_gids)/sizeof(root_gids[0]))); assertEqualInt(0, owner_parse(ROOT ".", &uid, &gid)); - assertEqualInt(ROOT_UID, uid); -#if defined(__CYGWIN__) - { - int gidIsOneOf = (ROOT_GID1 == gid) - || (ROOT_GID2 == gid) - || (ROOT_GID3 == gid); - assertEqualInt(1, gidIsOneOf); - } -#else - assertEqualInt(ROOT_GID, gid); -#endif + assert(int_in_list(uid, root_uids, + sizeof(root_uids)/sizeof(root_uids[0]))); + assert(int_in_list(gid, root_gids, + sizeof(root_gids)/sizeof(root_gids[0]))); + + assertEqualInt(0, owner_parse("111", &uid, &gid)); + assertEqualInt(111, uid); + assertEqualInt(-1, gid); + + assertEqualInt(0, owner_parse("112:", &uid, &gid)); + assertEqualInt(112, uid); + /* Can't assert gid, since we don't know gid for user #112. */ + + assertEqualInt(0, owner_parse("113.", &uid, &gid)); + assertEqualInt(113, uid); + /* Can't assert gid, since we don't know gid for user #113. */ + + assertEqualInt(0, owner_parse(":114", &uid, &gid)); + assertEqualInt(-1, uid); + assertEqualInt(114, gid); + + assertEqualInt(0, owner_parse(".115", &uid, &gid)); + assertEqualInt(-1, uid); + assertEqualInt(115, gid); + + assertEqualInt(0, owner_parse("116:117", &uid, &gid)); + assertEqualInt(116, uid); + assertEqualInt(117, gid); /* * TODO: Lookup current user/group name, build strings and @@ -104,6 +121,7 @@ DEFINE_TEST(test_owner_parse) * Alternatively, redirect stderr temporarily to suppress the output. */ + cpio_progname = "Ignore this message"; assertEqualInt(1, owner_parse(":nonexistentgroup", &uid, &gid)); assertEqualInt(1, owner_parse(ROOT ":nonexistentgroup", &uid, &gid)); assertEqualInt(1,
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200907060202.n6622jw9065554>