Module Name:    src
Committed By:   kre
Date:           Sat Sep 21 20:48:51 UTC 2024

Modified Files:
        src/bin/sh: options.c sh.1
        src/tests/bin/sh: t_shift.sh

Log Message:
In !TINYPROG versions of sh, make the builtin "shift" builtin command
perform a rotate instead or shift if given a numeric arg (which is
otherwise an error).

"set -- a b c; shift -1; echo $*" will echo "c a b".

While here, make the shift builtin comply with POSIX, and accept
(and ignore) a '--' arg before the shift (or rotate) count.

Document the variant of shift in sh(1)

Adapt the shell test for shift to not expect "shift -1" to be an
error, test that rotate works as expected, and include some tests
that use the (useless, but required) "--" arg.


To generate a diff of this commit:
cvs rdiff -u -r1.59 -r1.60 src/bin/sh/options.c
cvs rdiff -u -r1.263 -r1.264 src/bin/sh/sh.1
cvs rdiff -u -r1.2 -r1.3 src/tests/bin/sh/t_shift.sh

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/bin/sh/options.c
diff -u src/bin/sh/options.c:1.59 src/bin/sh/options.c:1.60
--- src/bin/sh/options.c:1.59	Fri Jul 12 07:30:30 2024
+++ src/bin/sh/options.c	Sat Sep 21 20:48:50 2024
@@ -1,4 +1,4 @@
-/*	$NetBSD: options.c,v 1.59 2024/07/12 07:30:30 kre Exp $	*/
+/*	$NetBSD: options.c,v 1.60 2024/09/21 20:48:50 kre Exp $	*/
 
 /*-
  * Copyright (c) 1991, 1993
@@ -37,7 +37,7 @@
 #if 0
 static char sccsid[] = "@(#)options.c	8.2 (Berkeley) 5/4/95";
 #else
-__RCSID("$NetBSD: options.c,v 1.59 2024/07/12 07:30:30 kre Exp $");
+__RCSID("$NetBSD: options.c,v 1.60 2024/09/21 20:48:50 kre Exp $");
 #endif
 #endif /* not lint */
 
@@ -415,17 +415,78 @@ freeparam(volatile struct shparam *param
  * The shift builtin command.
  */
 
+#ifndef TINY
+/* first the rotate variant */
+static inline int
+rotatecmd(int argc, char **argv)
+{
+	int n;
+	char **ap1, **ap2, **ss;
+
+	(void) nextopt(NULL);	/* ignore '--' as leading option */
+
+	/*
+	 * half this is just in case it ever becomes
+	 * a separate named command, while it remains
+	 * puerly an inline inside shift, the compiler
+	 * should optimise most of it to nothingness
+	 */
+	if (argptr[0] && argptr[1])
+		error("Usage: rotate [n]");
+	n = 1;
+	if (*argptr) {
+		if (**argptr == '-')
+			n = number(*argptr + 1);
+		else		/* anti-clockwise n == clockwise $# - n */
+			n = shellparam.nparam - number(*argptr);
+	}
+
+	if (n == 0 || n == shellparam.nparam)		/* nothing to do */
+		return 0;
+
+	if (n < 0 || n > shellparam.nparam)
+		error("can't rotate that many");
+
+	ap2 = ss = (char **)stalloc(n * sizeof(char *));
+	INTOFF;
+	for (ap1 = shellparam.p + shellparam.nparam - n;
+	     ap1 < shellparam.p + shellparam.nparam; )
+		*ap2++ = *ap1++;
+	for (ap2 = shellparam.p + shellparam.nparam, ap1 = ap2 - n;
+	     ap1 > shellparam.p; )
+		*--ap2 = *--ap1;
+	for (ap1 = ss + n; ap1 > ss; )
+		*--ap2 = *--ap1;
+	shellparam.optnext = NULL;
+	INTON;
+	stunalloc(ss);
+
+	return 0;
+}
+#endif
+
 int
 shiftcmd(int argc, char **argv)
 {
 	int n;
 	char **ap1, **ap2;
 
-	if (argc > 2)
+	(void) nextopt(NULL);	/* ignore '--' as leading option */
+
+	if (argptr[0] && argptr[1])
 		error("Usage: shift [n]");
+
+#ifndef TINY
+	if (*argptr && **argptr == '-') {
+		argptr = argv + 1;	/* reinit nextopt() */
+		optptr = NULL;
+		return rotatecmd(argc, argv);
+	}
+#endif
+
 	n = 1;
-	if (argc > 1)
-		n = number(argv[1]);
+	if (*argptr)
+		n = number(*argptr);
 	if (n > shellparam.nparam)
 		error("can't shift that many");
 	INTOFF;

Index: src/bin/sh/sh.1
diff -u src/bin/sh/sh.1:1.263 src/bin/sh/sh.1:1.264
--- src/bin/sh/sh.1:1.263	Sat Aug 10 19:21:32 2024
+++ src/bin/sh/sh.1	Sat Sep 21 20:48:50 2024
@@ -1,4 +1,4 @@
-.\"	$NetBSD: sh.1,v 1.263 2024/08/10 19:21:32 uwe Exp $
+.\"	$NetBSD: sh.1,v 1.264 2024/09/21 20:48:50 kre Exp $
 .\" Copyright (c) 1991, 1993
 .\"	The Regents of the University of California.  All rights reserved.
 .\"
@@ -3924,12 +3924,18 @@ parameters.)
 .\"
 .Pp
 .It Ic shift Op Ar n
-Shift the positional parameters
-.Ar n
-times.
 If
-.Ar n
+.Ar n ,
+which must be a decimal integer,
 is omitted, 1 is assumed.
+.Pp
+If
+.Ar n
+is unsigned
+.Pq or omitted
+shift the positional parameters
+.Ar n
+times.
 Each
 .Ic shift
 sets the value of
@@ -3944,12 +3950,81 @@ and so on, decreasing
 the value of
 .Li $#
 by one.
-The shift count must be less than or equal to the number of
-positional parameters (
-.Dq Li $# )
+The shift count
+.Pq Ar n
+must be less than or equal to the number of
+positional parameters
+.Pq Dq Li $#
 before the shift.
+A shift of
+.Li $#
+positions is equivalent to
+.Dq Li set\  Ns Fl Fl
+and results in unsetting all of the positional parameters
+and setting
+.Li $#
+to zero.
+The command
+.Dq Li shift\ 0
+does not alter
+.Li $#
+or any of the positional parameters.
+.Pp
+If
+.Ar n
+is negative, then the
+.Ic shift
+becomes a clockwise rotation,
+.Ar \&\-n
+times.
+The absolute value of
+.Ar n
+must be less than or equal to
+.Li $#
+.Pq the number of set positional parameters .
+Each rotation sets the value of
+.Li $1
+to the previous value of
+.Li ${$#}
+(if that were valid syntax) \(en the previous last positional parameter,
+sets
+.Li $2
+to the previous value of
+.Li $1 ,
+the value of
+.Li $3
+to the previous value of
+.Li $2 ,
+and so on, with the new last positional parameter becoming
+what was previously the penultimate positional parameter.
+The value of
+.Li $#
+is not altered.
+Shifts of
+.Li \&\-0
+and
+.Li \&\-$#
+are no-ops.
+The command sequence:
+.Dl shift\ \-n;\ shift\ n
+unsets the final
+.Ar n
+positional parameters.
+.Pp
+An anti-clockwise rotation of
+.Ar n
+places
+.Pq 0\ <=\  Ns Ar n Ns \ <=\ $#
+can be achieved using
+.Dq Li shift\ \-$(($#\ \-\ n)) .
+The command
+.Dq Li shift\ \-$(($#\ \-\ 1))
+is equivalent to, but much faster than:
+.Dl set Fl Fl \&\  Ns Do $@ Dc Do $1 Dc ; shift 1
 .\"
 .Pp
+The exit status is 0 if no error occurs, otherwise greater than 0.
+.Pp
 .It Ic specialvar Ar variable ...
 For each
 .Ar variable

Index: src/tests/bin/sh/t_shift.sh
diff -u src/tests/bin/sh/t_shift.sh:1.2 src/tests/bin/sh/t_shift.sh:1.3
--- src/tests/bin/sh/t_shift.sh:1.2	Tue May 17 09:05:14 2016
+++ src/tests/bin/sh/t_shift.sh	Sat Sep 21 20:48:50 2024
@@ -1,4 +1,4 @@
-# $NetBSD: t_shift.sh,v 1.2 2016/05/17 09:05:14 kre Exp $
+# $NetBSD: t_shift.sh,v 1.3 2024/09/21 20:48:50 kre Exp $
 #
 # Copyright (c) 2016 The NetBSD Foundation, Inc.
 # All rights reserved.
@@ -35,8 +35,10 @@ basic_shift_test_body() {
 
 	for a in			\
 	  "one-arg::0:one-arg"		\
+	  "one-arg:--:0:one-arg"	\
 	  "one-arg:1:0:one-arg"		\
-	  "one-arg:0:1 one-arg"		\
+	  "one-arg:-- 1:0:one-arg"	\
+	  "one-arg:-- 0:1 one-arg"	\
 	  "a b c::2 b c:a"		\
 	  "a b c:1:2 b c:a"		\
 	  "a b c:2:1 c:a:b"		\
@@ -68,6 +70,39 @@ basic_shift_test_body() {
     'set -- a b c d e;while [ $# -gt 0 ];do shift||echo ERR;done;echo complete'
 }
 
+atf_test_case basic_rotate
+basic_rotate_head() {
+	atf_set "descr" "Test correct operation of valid rotates"
+}
+basic_rotate_body() {
+
+	for a in			\
+	  "one-arg:-0:1 one-arg"	\
+	  "one-arg:-1:1 one-arg"	\
+	  "one-arg:-- -1:1 one-arg"	\
+	  "a b c:-1:3 c a b"		\
+	  "a b c:-2:3 b c a"		\
+	  "a b c:-3:3 a b c"		\
+	  "a b c:-- -3:3 a b c"		\
+	  "a b c:-0:3 a b c"		\
+	  "a b c d e f g h i j k l m n o:-1:15 o a b c d e f g h i j k l m n" \
+	  "a b c d e f g h i j k l m n o:-2:15 n o a b c d e f g h i j k l m" \
+	  "a b c d e f g h i j k l m n o:-9:15 g h i j k l m n o a b c d e f" \
+	  "a b c d e f g h i j k l m n o:-13:15 c d e f g h i j k l m n o a b" \
+	  "a b c d e f g h i j k l m n o:-14:15 b c d e f g h i j k l m n o a" \
+	  "a b c d e f g h i j k l m n o:-15:15 a b c d e f g h i j k l m n o"
+	do
+		oIFS="${IFS}"
+		IFS=:; set -- $a
+		IFS="${oIFS}"
+
+		init="$1"; n="$2"; res="$3"; shift 3
+
+		atf_check -s exit:0 -o "match:${res}" -e empty \
+			${TEST_SH} -c "set -- ${init}; shift $n;"' echo "$# $*"'
+	done
+}
+
 atf_test_case excessive_shift
 excessive_shift_head() {
 	atf_set "descr" "Test acceptable operation of shift too many"
@@ -115,6 +150,34 @@ excessive_shift_body() {
 	done
 }
 
+atf_test_case excessive_rotate
+excessive_rotate_head() {
+	atf_set "descr" "Test acceptable operation of rotate too many"
+}
+
+excessive_rotate_body() {
+	for a in				\
+		"one-arg:2"			\
+		"one-arg:4"			\
+		"one-arg:13"			\
+		"one two:3"			\
+		"one two:7"			\
+		"one two three four five:6"	\
+		"I II III IV V VI VII VIII IX X XI XII XIII XIV XV:16"	\
+		"I II III IV V VI VII VIII IX X XI XII XIII XIV XV:17"	\
+		"I II III IV V VI VII VIII IX X XI XII XIII XIV XV:30"	\
+		"I II III IV V VI VII VIII IX X XI XII XIII XIV XV:9999"
+	do
+		oIFS="${IFS}"
+		IFS=:; set -- $a
+		IFS="${oIFS}"
+
+		atf_check -s not-exit:0 -o match:OK -o not-match:ERR \
+			-e ignore ${TEST_SH} -c \
+			"set -- $1 ;echo OK:$#:-$2; shift -$2 && echo ERR"
+	done
+}
+
 atf_test_case function_shift
 function_shift_head() {
 	atf_set "descr" "Test that shift in a function does not affect outside"
@@ -149,10 +212,12 @@ non_numeric_shift_head() {
 non_numeric_shift_body() {
 
 	# there are 9 args set, 010 is 8 if parsed octal, 10 decimal
-	for a in a I 0x12 010 5V -1 ' ' '' +1 ' 1'
+	for a in a I 0x12 010 5V ' ' '' +1 ' 1'
 	do
 		atf_check -s not-exit:0 -o empty -e ignore ${TEST_SH} -c \
 			"set -- a b c d e f g h i; shift '$a' && echo ERROR"
+		atf_check -s not-exit:0 -o empty -e ignore ${TEST_SH} -c \
+			"set -- a b c d e f g h i; shift -- '$a' && echo ERROR"
 	done
 }
 
@@ -165,7 +230,7 @@ too_many_args_head() {
 too_many_args_body() {
 	# This tests the bug in PR bin/50896 is fixed
 
-	for a in "1 1" "1 0" "1 2 3" "1 foo" "1 --" "-- 1"
+	for a in "1 1" "1 0" "1 2 3" "1 foo" "1 --" "-- 1 2" "-1 2"
 	do
 		atf_check -s not-exit:0 -o empty -e not-empty ${TEST_SH} -c \
 			" set -- a b c d; shift ${a} ; echo FAILED "
@@ -178,4 +243,6 @@ atf_init_test_cases() {
 	atf_add_test_case function_shift
 	atf_add_test_case non_numeric_shift
 	atf_add_test_case too_many_args
+	atf_add_test_case basic_rotate
+	atf_add_test_case excessive_rotate
 }

Reply via email to