chown: support -H -L -P if ENABLE_DESKTOP

chmod: cosmetic fixes
expr: smallish help for dumb compilers

3 files changed, 109 insertions, 28 deletions

diff --git a/coreutils/chmod.c b/coreutils/chmod.c
index 74788bbbe..11c6731a1 100644
--- a/coreutils/chmod.c
+++ b/coreutils/chmod.c
@@ -118,10 +118,12 @@ int chmod_main(int argc, char **argv)
 /*
 Security: chmod is too important and too subtle.
 This is a test script (busybox chmod versus coreutils).
-Run it in empty dir. Probably requires bash.
+Run it in empty directory.
 
 #!/bin/sh
-function create() {
+t1="/tmp/busybox chmod"
+t2="/usr/bin/chmod"
+create() {
     rm -rf $1; mkdir $1
     (
     cd $1 || exit 1
@@ -134,17 +136,16 @@ function create() {
     ln -s ../up dir/up
     )
 }
-function tst() {
+tst() {
     (cd test1; $t1 $1)
     (cd test2; $t2 $1)
     (cd test1; ls -lR) >out1
     (cd test2; ls -lR) >out2
     echo "chmod $1" >out.diff
     if ! diff -u out1 out2 >>out.diff; then exit 1; fi
-    mv out.diff out1.diff
+    rm out.diff
 }
-t1="/tmp/busybox chmod"
-t2="/usr/bin/chmod"
+echo "If script produced 'out.diff' file, then at least one testcase failed"
 create test1; create test2
 tst "a+w file"
 tst "a-w dir"
diff --git a/coreutils/chown.c b/coreutils/chown.c
index 3380677bc..439b62ac8 100644
--- a/coreutils/chown.c
+++ b/coreutils/chown.c
@@ -17,25 +17,31 @@ static struct bb_uidgid_t ugid = { -1, -1 };
 
 static int (*chown_func)(const char *, uid_t, gid_t) = chown;
 
+#define OPT_STR     ("Rh" USE_DESKTOP("vcfLHP"))
+#define BIT_RECURSE 1
+#define BIT_NODEREF 2
+#define BIT_TRAVERSE 0x20
+#define BIT_TRAVERSETOP (0x20|0x40)
 #define OPT_RECURSE (option_mask32 & 1)
 #define OPT_NODEREF (option_mask32 & 2)
-#define OPT_VERBOSE (USE_DESKTOP(option_mask32 & 4) SKIP_DESKTOP(0))
-#define OPT_CHANGED (USE_DESKTOP(option_mask32 & 8) SKIP_DESKTOP(0))
+#define OPT_VERBOSE (USE_DESKTOP(option_mask32 & 0x04) SKIP_DESKTOP(0))
+#define OPT_CHANGED (USE_DESKTOP(option_mask32 & 0x08) SKIP_DESKTOP(0))
 #define OPT_QUIET   (USE_DESKTOP(option_mask32 & 0x10) SKIP_DESKTOP(0))
-#define OPT_STR     ("Rh" USE_DESKTOP("vcf"))
-
-/* TODO:
- * -H if a command line argument is a symbolic link to a directory, traverse it
+/* POSIX options
  * -L traverse every symbolic link to a directory encountered
+ * -H if a command line argument is a symbolic link to a directory, traverse it
  * -P do not traverse any symbolic links (default)
- */
+ * We do not conform to the following:
+ * "Specifying more than one of -H, -L, and -P is not an error.
+ * The last option specified shall determine the behavior of the utility." */
+/* -L */
+#define OPT_TRAVERSE    (USE_DESKTOP(option_mask32 & BIT_TRAVERSE) SKIP_DESKTOP(0))
+/* -H or -L */
+#define OPT_TRAVERSETOP (USE_DESKTOP(option_mask32 & BIT_TRAVERSETOP) SKIP_DESKTOP(0))
 
 static int fileAction(const char *fileName, struct stat *statbuf,
 		void ATTRIBUTE_UNUSED *junk, int depth)
 {
-	// TODO: -H/-L/-P
-	// if (depth ... && S_ISLNK(statbuf->st_mode)) ....
-
 	if (!chown_func(fileName,
 			(ugid.uid == (uid_t)-1) ? statbuf->st_uid : ugid.uid,
 			(ugid.gid == (gid_t)-1) ? statbuf->st_gid : ugid.gid)
@@ -62,16 +68,31 @@ int chown_main(int argc, char **argv)
 	getopt32(argc, argv, OPT_STR);
 	argv += optind;
 
-	if (OPT_NODEREF) chown_func = lchown;
+	/* This matches coreutils behavior (almost - see below) */
+	if (OPT_NODEREF
+	    /* || (OPT_RECURSE && !OPT_TRAVERSETOP): */
+	    USE_DESKTOP( || (option_mask32 & (BIT_RECURSE|BIT_TRAVERSETOP)) == BIT_RECURSE)
+	) {
+		chown_func = lchown;
+	}
 
 	parse_chown_usergroup_or_die(&ugid, argv[0]);
 
 	/* Ok, ready to do the deed now */
 	argv++;
 	do {
-		if (!recursive_action(*argv,
-				OPT_RECURSE,	// recurse
-				FALSE,          // follow links: TODO: -H/-L/-P
+		char *arg = *argv;
+
+		if (OPT_TRAVERSETOP) {
+			/* resolves symlink (even recursive) */
+			arg = xmalloc_realpath(arg);
+			if (!arg)
+				continue;
+		}
+
+		if (!recursive_action(arg,
+				OPT_RECURSE,    // recurse
+				OPT_TRAVERSE,   // follow links if -L
 				FALSE,          // depth first
 				fileAction,     // file action
 				fileAction,     // dir action
@@ -80,7 +101,66 @@ int chown_main(int argc, char **argv)
 		) {
 			retval = EXIT_FAILURE;
 		}
+
+		if (OPT_TRAVERSETOP)
+			free(arg);
 	} while (*++argv);
 
 	return retval;
 }
+
+/*
+Testcase. Run in empty directory.
+
+#!/bin/sh
+t1="/tmp/busybox chown"
+t2="/usr/bin/chown"
+create() {
+    rm -rf $1; mkdir $1
+    (
+    cd $1 || exit 1
+    mkdir dir dir2
+    >up
+    >file
+    >dir/file
+    >dir2/file
+    ln -s dir linkdir
+    ln -s file linkfile
+    ln -s ../up dir/linkup
+    ln -s ../dir2 dir/linkupdir2
+    )
+    chown -R 0:0 $1
+}
+tst() {
+    create test1
+    create test2
+    (cd test1; $t1 $1)
+    (cd test2; $t2 $1)
+    (cd test1; ls -lnR) >out1
+    (cd test2; ls -lnR) >out2
+    echo "chown $1" >out.diff
+    if ! diff -u out1 out2 >>out.diff; then exit 1; fi
+    rm out.diff
+}
+tst_for_each() {
+    tst "$1 1:1 file"
+    tst "$1 1:1 dir"
+    tst "$1 1:1 linkdir"
+    tst "$1 1:1 linkfile"
+}
+echo "If script produced 'out.diff' file, then at least one testcase failed"
+# These match coreutils 6.8:
+tst_for_each ""
+tst_for_each "-R"
+tst_for_each "-RP"
+tst_for_each "-RL"
+tst_for_each "-RH"
+tst_for_each "-h"
+tst_for_each "-hR"
+tst_for_each "-hRP"
+# Below: with "chown linkdir" coreutils 6.8 will chown linkdir _target_,
+# we lchown _the link_. I believe we are "more correct".
+#tst_for_each "-hRL"
+#tst_for_each "-hRH"
+
+*/
diff --git a/coreutils/expr.c b/coreutils/expr.c
index 62d1f3a8f..b7e33de49 100644
--- a/coreutils/expr.c
+++ b/coreutils/expr.c
@@ -136,8 +136,8 @@ static int null(VALUE * v)
 {
 	if (v->type == integer)
 		return v->u.i == 0;
-	else				/* string: */
-		return v->u.s[0] == '\0' || LONE_CHAR(v->u.s, '0');
+	/* string: */
+	return v->u.s[0] == '\0' || LONE_CHAR(v->u.s, '0');
 }
 
 /* Coerce V to a string value (can't fail).  */
@@ -194,16 +194,16 @@ static int cmp_common(VALUE * l, VALUE * r, int op)
 		cmpval = l->u.i - r->u.i;
 	if (op == '<')
 		return cmpval < 0;
-	else if (op == ('L' + 'E'))
+	if (op == ('L' + 'E'))
 		return cmpval <= 0;
-	else if (op == '=')
+	if (op == '=')
 		return cmpval == 0;
-	else if (op == '!')
+	if (op == '!')
 		return cmpval != 0;
-	else if (op == '>')
+	if (op == '>')
 		return cmpval > 0;
-	else				/* >= */
-		return cmpval >= 0;
+	/* >= */
+	return cmpval >= 0;
 }
 
 /* The arithmetic operator handling functions.  */