Audit chgrp(1)

1) Refactor manpage so it's actually fun to read.
2) BUGFIX: Call (l)chown properly when the H-flag is specified
   (only when depth > 0).
3) BUGFIX: Call (l)chown properly when the h-flag is specified
   (only when depth = 0).
4) BUGFIX: Only recurse() in chgrp() when the initial chownf()
   succeeds.
5) Style fixes, argv-basing.
6) Rename status to ret for consistency.
7) Add blank line before return.

README

@@ -12,7 +12,7 @@ The following tools are implemented ('*' == finished, '#' == UTF-8 support,
 =*| basename        yes                          none
 =*| cal             yes                          none
 =*| cat             yes                          none
-=*  chgrp           yes                          none
+=*| chgrp           yes                          none
 =*| chmod           yes                          none
 =*  chown           yes                          none
 =*| chroot          non-posix                    none

chgrp.1

@@ -1,9 +1,9 @@
-.Dd February 9, 2015
+.Dd March 8, 2015
 .Dt CHGRP 1
 .Os sbase
 .Sh NAME
 .Nm chgrp
-.Nd change the file group ownership
+.Nd change file group ownership
 .Sh SYNOPSIS
 .Nm
 .Op Fl h
@@ -12,29 +12,29 @@
 .Op Fl H | L | P
 .Oc
 .Ar group
-.Ar file...
+.Ar file ...
 .Sh DESCRIPTION
 .Nm
-sets the group id of the files specified by
+sets the group id of each
 .Ar file
-to the gid of the group named
+to the gid of
 .Ar group .
 .Sh OPTIONS
 .Bl -tag -width Ds
 .It Fl h
-Change the group ID of the symlink itself.  This flag cannot be used
+Preserve
-with
+.Ar file
-.Op Fl R .
+if it is a symbolic link.
 .It Fl R
-Change file group ownership recursively.
+Change file group ownerships recursively.
 .It Fl H
-Only dereference symbolic links that are passed as command line arguments when
+Dereference
-recursively traversing directories.
+.Ar file
+if it is a symbolic link.
 .It Fl L
-Always dereference symbolic links while recursively traversing directories.
+Dereference all symbolic links.
 .It Fl P
-Don't dereference symbolic links (default).
+Preserve symbolic links. This is the default.
-.Ar file .
 .El
 .Sh SEE ALSO
 .Xr chmod 1 ,

chgrp.c

@@ -8,21 +8,31 @@
 #include "util.h"
 
 static int gid;
-static int status;
+static int ret = 0;
-static int Rflag;
+static int hflag = 0;
+static int Rflag = 0;
 static struct stat st;
-static char *chownf_name = "chown";
-static int (*chownf)(const char *, uid_t, gid_t) = chown;
 
 static void
 chgrp(const char *path, int depth)
 {
+	char *chownf_name;
+	int (*chownf)(const char *, uid_t, gid_t);
+
+	if (recurse_follow == 'P' || (recurse_follow == 'H' && depth) || (hflag && !depth)) {
+		chownf_name = "lchown";
+		chownf = lchown;
+	} else {
+		chownf_name = "chown";
+		chownf = chown;
+	}
+
 	if (chownf(path, st.st_uid, gid) < 0) {
 		weprintf("%s %s:", chownf_name, path);
-		status = 1;
+		ret = 1;
-	}
+	} else if (Rflag) {
-	if (Rflag)
 		recurse(path, chgrp, depth);
+	}
 }
 
 static void
@@ -38,8 +48,7 @@ main(int argc, char *argv[])
 
 	ARGBEGIN {
 	case 'h':
-		chownf_name = "lchown";
+		hflag = 1;
-		chownf = lchown;
 		break;
 	case 'R':
 		Rflag = 1;
@@ -55,14 +64,9 @@ main(int argc, char *argv[])
 
 	if (argc < 2)
 		usage();
-	if (recurse_follow == 'P') {
-		chownf_name = "lchown";
-		chownf = lchown;
-	}
 
 	errno = 0;
-	gr = getgrnam(argv[0]);
+	if (!(gr = getgrnam(argv[0]))) {
-	if (!gr) {
 		if (errno)
 			eprintf("getgrnam %s:", argv[0]);
 		else
@@ -70,13 +74,14 @@ main(int argc, char *argv[])
 	}
 	gid = gr->gr_gid;
 
-	while (*++argv) {
+	for (; *argv; argc--, argv++) {
 		if (stat(*argv, &st) < 0) {
 			weprintf("stat %s:", *argv);
-			status = 1;
+			ret = 1;
 			continue;
 		}
 		chgrp(*argv, 0);
 	}
-	return status;
+
+	return ret;
 }