Re: RFC: change chown *not* to look up numeric user/group names

[Jim Meyering]

Jim Meyering <address@hidden> wrote:
...
> I propose to change GNU chown to perform that look-up of an all-numeric
> "user" or "group" string only when the POSIXLY_CORRECT envvar is set.
> Otherwise, (when POSIXLY_CORRECT is not set and a "name" is a valid user
> ID or group ID), chown would use the value obtained from converting the
> string with a function like strtoul.
>
> For consistency, the same policy would apply to chgrp.

Here's a postscript to the discussion that started above, aka,

    http://thread.gmane.org/gmane.comp.gnu.coreutils.bugs/8682

Today I realized that there is a way to avoid the look-up
without resorting to a new option.
The trick is to realize that in commands like the following

   chown +0:+0 file
   chgrp +0 file

the getpwnam and getgrnam look-up operations must always
fail, since "+" is not a valid character in a user name.
Hence, chown and chgrp can skip those function calls when
the first byte is "+".  chown and chgrp have always accepted
a leading sign on those arguments.

So if you want to avoid the expense of what you deem to be a pointless
name-to-ID look-up of "0" (or any numeric value), just prepend a "+" --
if you're using the patch below.

Here's a demo:

The old way:

    # strace -e open,chown ./chown 0:0 /t/Z
    open("/etc/ld.so.cache", O_RDONLY)      = 3
    open("/lib/libc.so.6", O_RDONLY)        = 3
    open("/etc/nsswitch.conf", O_RDONLY)    = 3
    open("/etc/ld.so.cache", O_RDONLY)      = 3
    open("/lib/libnss_compat.so.2", O_RDONLY) = 3
    open("/lib/libnsl.so.1", O_RDONLY)      = 3
    open("/etc/ld.so.cache", O_RDONLY)      = 3
    open("/lib/libnss_nis.so.2", O_RDONLY)  = 3
    open("/lib/libnss_files.so.2", O_RDONLY) = 3
    open("/etc/passwd", O_RDONLY)           = 3
    open("/etc/group", O_RDONLY)            = 3
    open(".", O_RDONLY)                     = 3
    chown("/t/Z", 0, 0)                     = 0
    Process 17271 detached

New way: no look-up:

    # strace -e open,chown ./chown +0:+0 /t/Z
    open("/etc/ld.so.cache", O_RDONLY)      = 3
    open("/lib/libc.so.6", O_RDONLY)        = 3
    open(".", O_RDONLY)                     = 3
    chown("/t/Z", 0, 0)                     = 0
    Process 17234 detached

However, there is a drawback: this syntax is not portable.
For example, Solaris 10's chgrp and chown reject it, e.g.,

    $ /bin/chgrp +0 k
    chgrp: unknown group: +0
    [Exit 2]

Well, seeing that, I'm less enthusiastic about this change.
I'll sleep on it.  If I do apply it, I'll also document that
GNU chown and chgrp accept this syntax, and the semantics.

Here's the proposed gnulib patch:

2007-01-19  Jim Meyering  <address@hidden>

        * lib/userspec.c (parse_with_separator): If a user or group string
        starts with "+", skip the corresponding name-to-ID look-up, since
        such a look-up must fail: user and group names may not include "+".

Index: lib/userspec.c
===================================================================
RCS file: /sources/gnulib/gnulib/lib/userspec.c,v
retrieving revision 1.53
diff -u -p -r1.53 userspec.c
--- lib/userspec.c      13 Sep 2006 22:38:14 -0000      1.53
+++ lib/userspec.c      19 Jan 2007 22:38:12 -0000
@@ -1,5 +1,5 @@
 /* userspec.c -- Parse a user and group string.
-   Copyright (C) 1989-1992, 1997-1998, 2000, 2002-2006 Free Software
+   Copyright (C) 1989-1992, 1997-1998, 2000, 2002-2007 Free Software
    Foundation, Inc.

    This program is free software; you can redistribute it and/or modify
@@ -156,7 +156,8 @@ parse_with_separator (char const *spec,

   if (u != NULL)
     {
-      pwd = getpwnam (u);
+      /* If it starts with "+", skip the look-up.  */
+      pwd = (*u == '+' ? NULL : getpwnam (u));
       if (pwd == NULL)
        {
          bool use_login_group = (separator != NULL && g == NULL);
@@ -196,7 +197,8 @@ parse_with_separator (char const *spec,
   if (g != NULL && error_msg == NULL)
     {
       /* Explicit group.  */
-      grp = getgrnam (g);
+      /* If it starts with "+", skip the look-up.  */
+      grp = (*g == '+' ? NULL : getgrnam (g));
       if (grp == NULL)
        {
          unsigned long int tmp;