From owner-svn-src-head@FreeBSD.ORG  Fri Jul  3 19:17:53 2009
Return-Path: <owner-svn-src-head@FreeBSD.ORG>
Delivered-To: svn-src-head@freebsd.org
Received: by hub.freebsd.org (Postfix, from userid 1033)
	id 59EF31065672; Fri,  3 Jul 2009 19:17:53 +0000 (UTC)
Date: Fri, 3 Jul 2009 19:17:53 +0000
From: Alexey Dokuchaev <danfe@FreeBSD.org>
To: Tim Kientzle <kientzle@FreeBSD.org>
Message-ID: <20090703191753.GA48087@FreeBSD.org>
References: <200907031754.n63HsXRn084493@svn.freebsd.org>
Mime-Version: 1.0
Content-Type: text/plain; charset=koi8-r
Content-Disposition: inline
In-Reply-To: <200907031754.n63HsXRn084493@svn.freebsd.org>
User-Agent: Mutt/1.4.2.1i
Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org,
	src-committers@freebsd.org
Subject: Re: svn commit: r195318 - head/usr.bin/cpio
X-BeenThere: svn-src-head@freebsd.org
X-Mailman-Version: 2.1.5
Precedence: list
List-Id: SVN commit messages for the src tree for head/-current
	<svn-src-head.freebsd.org>
List-Unsubscribe: <http://lists.freebsd.org/mailman/listinfo/svn-src-head>,
	<mailto:svn-src-head-request@freebsd.org?subject=unsubscribe>
List-Archive: <http://lists.freebsd.org/pipermail/svn-src-head>
List-Post: <mailto:svn-src-head@freebsd.org>
List-Help: <mailto:svn-src-head-request@freebsd.org?subject=help>
List-Subscribe: <http://lists.freebsd.org/mailman/listinfo/svn-src-head>,
	<mailto:svn-src-head-request@freebsd.org?subject=subscribe>
X-List-Received-Date: Fri, 03 Jul 2009 19:17:53 -0000

Tim Kientzle wrote:
> Author: kientzle
> Date: Fri Jul  3 17:54:33 2009
> New Revision: 195318
> URL: http://svn.freebsd.org/changeset/base/195318
> 
> + * Where uid/gid are decimal representations and groupname/username
> + * are names to be looked up in system database.  Note that ^^^^^^^

Bad indentation: "uid/gid" occupies seven characters, so wrap is
premature here (there are exactly seven ^ marks above).

> + * 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.
>   *
>   * A period can be used instead of the colon.
>   *
> - * Sets uid/gid as appropriate, -1 indicates uid/gid not specified.
> + * 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);
> +}

Roll own your own strtoul(3)?  What kind of NIH is this?  :-)

> +
>  int
>  owner_parse(const char *spec, int *uid, int *gid)
>  {
> @@ -318,24 +338,34 @@ owner_parse(const char *spec, int *uid, 
>  		}
>  		memcpy(user, u, ue - u);
>  		user[ue - u] = '\0';
> -		pwent = getpwnam(user);
> -		if (pwent == NULL) {
> -			cpio_warnc(errno, "Couldn't lookup user ``%s''", user);
> -			return (1);
> +		*uid = decimal_parse(user);
> +		if (*uid < 0) {
> +			/* Couldn't parse as integer, try username lookup. */
> +			pwent = getpwnam(user);
> +			if (pwent == NULL) {
> +				cpio_warnc(errno,
> +				    "Couldn't lookup user ``%s''", user);
> +				return (1);
> +			}
> +			*uid = pwent->pw_uid;
> +			if (*ue != '\0' && *g == '\0')
> +				*gid = pwent->pw_gid;

Why not something simple like this:

	if ((pwent = getpwnam(user)) != NULL)
		uid = pwent->pw_uid;
	else {
		errno = 0;
		uid = strtoul(user, &ep, 10);
		if (errno || user[0] == '\0' || *ep != '\0')
			cpio_warnc("invalid user: %s", user);
	}

./danfe