netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH iproute2 1/2] configure: avoid un-recommended command substitution form
@ 2023-12-18  3:30 Eli Schwartz
  2023-12-18  3:30 ` [PATCH iproute2 2/2] configure: use the portable printf to suppress newlines in messages Eli Schwartz
  2023-12-28  0:46 ` [PATCH iproute2 1/2] configure: avoid un-recommended command substitution form Stephen Hemminger
  0 siblings, 2 replies; 11+ messages in thread
From: Eli Schwartz @ 2023-12-18  3:30 UTC (permalink / raw)
  To: netdev

The use of backticks to surround commands instead of "$(cmd)" is a
legacy of the oldest pre-POSIX shells. It is confusing, unreliable, and
hard to read. Its use is not recommended in new programs.

See: http://mywiki.wooledge.org/BashFAQ/082
---
 configure | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/configure b/configure
index eb689341..19845f3c 100755
--- a/configure
+++ b/configure
@@ -270,8 +270,8 @@ check_elf()
 	echo "HAVE_ELF:=y" >>$CONFIG
 	echo "yes"
 
-	echo 'CFLAGS += -DHAVE_ELF' `${PKG_CONFIG} libelf --cflags` >> $CONFIG
-	echo 'LDLIBS += ' `${PKG_CONFIG} libelf --libs` >>$CONFIG
+	echo 'CFLAGS += -DHAVE_ELF' "$(${PKG_CONFIG} libelf --cflags)" >> $CONFIG
+	echo 'LDLIBS += ' "$(${PKG_CONFIG} libelf --libs)" >>$CONFIG
     else
 	echo "no"
     fi
@@ -389,8 +389,8 @@ check_selinux()
 		echo "HAVE_SELINUX:=y" >>$CONFIG
 		echo "yes"
 
-		echo 'LDLIBS +=' `${PKG_CONFIG} --libs libselinux` >>$CONFIG
-		echo 'CFLAGS += -DHAVE_SELINUX' `${PKG_CONFIG} --cflags libselinux` >>$CONFIG
+		echo 'LDLIBS +=' "$(${PKG_CONFIG} --libs libselinux)" >>$CONFIG
+		echo 'CFLAGS += -DHAVE_SELINUX' "$(${PKG_CONFIG} --cflags libselinux)" >>$CONFIG
 	else
 		echo "no"
 	fi
@@ -402,8 +402,8 @@ check_tirpc()
 		echo "HAVE_RPC:=y" >>$CONFIG
 		echo "yes"
 
-		echo 'LDLIBS +=' `${PKG_CONFIG} --libs libtirpc` >>$CONFIG
-		echo 'CFLAGS += -DHAVE_RPC' `${PKG_CONFIG} --cflags libtirpc` >>$CONFIG
+		echo 'LDLIBS +=' "$(${PKG_CONFIG} --libs libtirpc)" >>$CONFIG
+		echo 'CFLAGS += -DHAVE_RPC' "$(${PKG_CONFIG} --cflags libtirpc)" >>$CONFIG
 	else
 		echo "no"
 	fi
@@ -415,8 +415,8 @@ check_mnl()
 		echo "HAVE_MNL:=y" >>$CONFIG
 		echo "yes"
 
-		echo 'CFLAGS += -DHAVE_LIBMNL' `${PKG_CONFIG} libmnl --cflags` >>$CONFIG
-		echo 'LDLIBS +=' `${PKG_CONFIG} libmnl --libs` >> $CONFIG
+		echo 'CFLAGS += -DHAVE_LIBMNL' "$(${PKG_CONFIG} libmnl --cflags)" >>$CONFIG
+		echo 'LDLIBS +=' "$(${PKG_CONFIG} libmnl --libs)" >> $CONFIG
 	else
 		echo "no"
 	fi
@@ -456,8 +456,8 @@ EOF
 	echo "no"
     else
 	if ${PKG_CONFIG} libbsd --exists; then
-		echo 'CFLAGS += -DHAVE_LIBBSD' `${PKG_CONFIG} libbsd --cflags` >>$CONFIG
-		echo 'LDLIBS +=' `${PKG_CONFIG} libbsd --libs` >> $CONFIG
+		echo 'CFLAGS += -DHAVE_LIBBSD' "$(${PKG_CONFIG} libbsd --cflags)" >>$CONFIG
+		echo 'LDLIBS +=' "$(${PKG_CONFIG} libbsd --libs)" >> $CONFIG
 		echo "no"
 	else
 		echo 'CFLAGS += -DNEED_STRLCPY' >>$CONFIG
@@ -473,8 +473,8 @@ check_cap()
 		echo "HAVE_CAP:=y" >>$CONFIG
 		echo "yes"
 
-		echo 'CFLAGS += -DHAVE_LIBCAP' `${PKG_CONFIG} libcap --cflags` >>$CONFIG
-		echo 'LDLIBS +=' `${PKG_CONFIG} libcap --libs` >> $CONFIG
+		echo 'CFLAGS += -DHAVE_LIBCAP' "$(${PKG_CONFIG} libcap --cflags)" >>$CONFIG
+		echo 'LDLIBS +=' "$(${PKG_CONFIG} libcap --libs)" >> $CONFIG
 	else
 		echo "no"
 	fi
-- 
2.41.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH iproute2 2/2] configure: use the portable printf to suppress newlines in messages
  2023-12-18  3:30 [PATCH iproute2 1/2] configure: avoid un-recommended command substitution form Eli Schwartz
@ 2023-12-18  3:30 ` Eli Schwartz
  2023-12-28  0:46   ` Stephen Hemminger
  2023-12-28  0:46 ` [PATCH iproute2 1/2] configure: avoid un-recommended command substitution form Stephen Hemminger
  1 sibling, 1 reply; 11+ messages in thread
From: Eli Schwartz @ 2023-12-18  3:30 UTC (permalink / raw)
  To: netdev

Per https://pubs.opengroup.org/onlinepubs/9699919799/utilities/echo.html
the "echo" utility is un-recommended and its behavior is non-portable
and unpredictable. It *should* be marked as obsolescent, but was not,
due solely "because of its extremely widespread use in historical
applications".

POSIX doesn't require the -n option, and although its behavior is
reliable in `#!/bin/bash` scripts, this configure script uses
`#!/bin/sh` and cannot rely on echo -n.

The use of printf even without newline suppression or backslash
character sequences is nicer for consistency, since there are a variety
of ways it can go wrong with echo including "echoing the value of a
shell or environment variable".

See:
https://pubs.opengroup.org/onlinepubs/9699919799/utilities/echo.html
https://cfajohnson.com/shell/cus-faq.html#Q0b
---
 configure | 223 +++++++++++++++++++++++++++---------------------------
 1 file changed, 113 insertions(+), 110 deletions(-)

diff --git a/configure b/configure
index 19845f3c..5d8d9ca5 100755
--- a/configure
+++ b/configure
@@ -20,10 +20,12 @@ check_toolchain()
     : ${AR=ar}
     : ${CC=gcc}
     : ${YACC=bison}
-    echo "PKG_CONFIG:=${PKG_CONFIG}" >>$CONFIG
-    echo "AR:=${AR}" >>$CONFIG
-    echo "CC:=${CC}" >>$CONFIG
-    echo "YACC:=${YACC}" >>$CONFIG
+    {
+        printf '%s\n' "PKG_CONFIG:=${PKG_CONFIG}"
+        printf '%s\n' "AR:=${AR}"
+        printf '%s\n' "CC:=${CC}"
+        printf '%s\n' "YACC:=${YACC}"
+    } >>$CONFIG
 }
 
 check_atm()
@@ -38,10 +40,10 @@ int main(int argc, char **argv) {
 EOF
 
     if $CC -I$INCLUDE -o $TMPDIR/atmtest $TMPDIR/atmtest.c -latm >/dev/null 2>&1; then
-	echo "TC_CONFIG_ATM:=y" >>$CONFIG
-	echo yes
+	printf '%s\n' "TC_CONFIG_ATM:=y" >>$CONFIG
+	printf '%s\n' yes
     else
-	echo no
+	printf '%s\n' no
     fi
     rm -f $TMPDIR/atmtest.c $TMPDIR/atmtest
 }
@@ -49,7 +51,7 @@ EOF
 check_xtables()
 {
 	if ! ${PKG_CONFIG} xtables --exists; then
-		echo "TC_CONFIG_NO_XT:=y" >>$CONFIG
+		printf '%s\n' "TC_CONFIG_NO_XT:=y" >>$CONFIG
 	fi
 }
 
@@ -77,8 +79,8 @@ EOF
 
     if $CC -I$INCLUDE $IPTC -o $TMPDIR/ipttest $TMPDIR/ipttest.c $IPTL \
 	$(${PKG_CONFIG} xtables --cflags --libs) -ldl >/dev/null 2>&1; then
-	echo "TC_CONFIG_XT:=y" >>$CONFIG
-	echo "using xtables"
+	printf '%s\n' "TC_CONFIG_XT:=y" >>$CONFIG
+	printf '%s\n' "using xtables"
     fi
     rm -f $TMPDIR/ipttest.c $TMPDIR/ipttest
 }
@@ -111,8 +113,8 @@ int main(int argc, char **argv) {
 EOF
 
     if $CC -I$INCLUDE $IPTC -o $TMPDIR/ipttest $TMPDIR/ipttest.c $IPTL -ldl >/dev/null 2>&1; then
-	echo "TC_CONFIG_XT_OLD:=y" >>$CONFIG
-	echo "using old xtables (no need for xt-internal.h)"
+	printf '%s\n' "TC_CONFIG_XT_OLD:=y" >>$CONFIG
+	printf '%s\n' "using old xtables (no need for xt-internal.h)"
     fi
     rm -f $TMPDIR/ipttest.c $TMPDIR/ipttest
 }
@@ -145,25 +147,24 @@ int main(int argc, char **argv) {
 
 EOF
 	if $CC -I$INCLUDE $IPTC -o $TMPDIR/ipttest $TMPDIR/ipttest.c $IPTL -ldl >/dev/null 2>&1; then
-	    echo "using old xtables with xt-internal.h"
-	    echo "TC_CONFIG_XT_OLD_H:=y" >>$CONFIG
+	    printf '%s\n' "using old xtables with xt-internal.h"
+	    printf '%s\n' "TC_CONFIG_XT_OLD_H:=y" >>$CONFIG
 	fi
 	rm -f $TMPDIR/ipttest.c $TMPDIR/ipttest
 }
 
 check_lib_dir()
 {
-	LIBDIR=$(echo $LIBDIR | sed "s|\${prefix}|$PREFIX|")
+	LIBDIR=$(printf '%s' "$LIBDIR" | sed "s|\${prefix}|$PREFIX|")
 
-	echo -n "lib directory: "
-	echo "$LIBDIR"
-	echo "LIBDIR:=$LIBDIR" >> $CONFIG
+	printf '%s\n' "lib directory: $LIBDIR"
+	printf '%s\n' "LIBDIR:=$LIBDIR" >> $CONFIG
 }
 
 check_ipt()
 {
 	if ! grep TC_CONFIG_XT $CONFIG > /dev/null; then
-		echo "using iptables"
+		printf '%s\n' "using iptables"
 	fi
 }
 
@@ -171,8 +172,8 @@ check_ipt_lib_dir()
 {
 	IPT_LIB_DIR=$(${PKG_CONFIG} --variable=xtlibdir xtables)
 	if [ -n "$IPT_LIB_DIR" ]; then
-		echo $IPT_LIB_DIR
-		echo "IPT_LIB_DIR:=$IPT_LIB_DIR" >> $CONFIG
+		printf '%s\n' "$IPT_LIB_DIR"
+		printf '%s\n' "IPT_LIB_DIR:=$IPT_LIB_DIR" >> $CONFIG
 		return
 	fi
 
@@ -180,13 +181,13 @@ check_ipt_lib_dir()
 		for file in "xtables" "iptables"; do
 			file="$dir/$file/lib*t_*so"
 			if [ -f $file ]; then
-				echo ${file%/*}
-				echo "IPT_LIB_DIR:=${file%/*}" >> $CONFIG
+				printf '%s\n' "${file%/*}"
+				printf '%s\n' "IPT_LIB_DIR:=${file%/*}" >> $CONFIG
 				return
 			fi
 		done
 	done
-	echo "not found!"
+	printf '%s\n' "not found!"
 }
 
 check_setns()
@@ -201,11 +202,11 @@ int main(int argc, char **argv)
 }
 EOF
     if $CC -I$INCLUDE -o $TMPDIR/setnstest $TMPDIR/setnstest.c >/dev/null 2>&1; then
-	echo "IP_CONFIG_SETNS:=y" >>$CONFIG
-	echo "yes"
-	echo "CFLAGS += -DHAVE_SETNS" >>$CONFIG
+	printf '%s\n' "IP_CONFIG_SETNS:=y" >>$CONFIG
+	printf '%s\n' "yes"
+	printf '%s\n' "CFLAGS += -DHAVE_SETNS" >>$CONFIG
     else
-	echo "no"
+	printf '%s\n' "no"
     fi
     rm -f $TMPDIR/setnstest.c $TMPDIR/setnstest
 }
@@ -226,10 +227,10 @@ int main(int argc, char **argv)
 }
 EOF
     if $CC -I$INCLUDE -o $TMPDIR/name_to_handle_at_test $TMPDIR/name_to_handle_at_test.c >/dev/null 2>&1; then
-        echo "yes"
-        echo "CFLAGS += -DHAVE_HANDLE_AT" >>$CONFIG
+        printf '%s\n' "yes"
+        printf '%s\n' "CFLAGS += -DHAVE_HANDLE_AT" >>$CONFIG
     else
-        echo "no"
+        printf '%s\n' "no"
     fi
     rm -f $TMPDIR/name_to_handle_at_test.c $TMPDIR/name_to_handle_at_test
 }
@@ -256,10 +257,10 @@ int main(void)
 EOF
 
     if $CC -I$INCLUDE -o $TMPDIR/ipsettest $TMPDIR/ipsettest.c >/dev/null 2>&1; then
-	echo "TC_CONFIG_IPSET:=y" >>$CONFIG
-	echo "yes"
+	printf '%s\n' "TC_CONFIG_IPSET:=y" >>$CONFIG
+	printf '%s\n' "yes"
     else
-	echo "no"
+	printf '%s\n' "no"
     fi
     rm -f $TMPDIR/ipsettest.c $TMPDIR/ipsettest
 }
@@ -267,13 +268,13 @@ EOF
 check_elf()
 {
     if ${PKG_CONFIG} libelf --exists; then
-	echo "HAVE_ELF:=y" >>$CONFIG
-	echo "yes"
+	printf '%s\n' "HAVE_ELF:=y" >>$CONFIG
+	printf '%s\n' "yes"
 
-	echo 'CFLAGS += -DHAVE_ELF' "$(${PKG_CONFIG} libelf --cflags)" >> $CONFIG
-	echo 'LDLIBS += ' "$(${PKG_CONFIG} libelf --libs)" >>$CONFIG
+	printf '%s\n' "CFLAGS += -DHAVE_ELF $(${PKG_CONFIG} libelf --cflags)" >> $CONFIG
+	printf '%s\n' "LDLIBS += $(${PKG_CONFIG} libelf --libs)" >>$CONFIG
     else
-	echo "no"
+	printf '%s\n' "no"
     fi
 }
 
@@ -320,7 +321,7 @@ check_force_libbpf_on()
     # if set LIBBPF_FORCE=on but no libbpf support, just exist the config
     # process to make sure we don't build without libbpf.
     if [ "$LIBBPF_FORCE" = on ]; then
-        echo "	LIBBPF_FORCE=on set, but couldn't find a usable libbpf"
+        printf '%s\n' "	LIBBPF_FORCE=on set, but couldn't find a usable libbpf"
         exit 1
     fi
 }
@@ -329,12 +330,12 @@ check_libbpf()
 {
     # if set LIBBPF_FORCE=off, disable libbpf entirely
     if [ "$LIBBPF_FORCE" = off ]; then
-        echo "no"
+        printf '%s\n' "no"
         return
     fi
 
     if ! ${PKG_CONFIG} libbpf --exists && [ -z "$LIBBPF_DIR" ] ; then
-        echo "no"
+        printf '%s\n' "no"
         check_force_libbpf_on
         return
     fi
@@ -356,69 +357,69 @@ check_libbpf()
     fi
 
     if ! have_libbpf_basic; then
-        echo "no"
-        echo "	libbpf version $LIBBPF_VERSION is too low, please update it to at least 0.1.0"
+        printf '%s\n' "no"
+        printf '%s\n' "	libbpf version $LIBBPF_VERSION is too low, please update it to at least 0.1.0"
         check_force_libbpf_on
         return
     else
-        echo "HAVE_LIBBPF:=y" >> $CONFIG
-        echo 'CFLAGS += -DHAVE_LIBBPF ' $LIBBPF_CFLAGS >> $CONFIG
-        echo "CFLAGS += -DLIBBPF_VERSION=\\\"$LIBBPF_VERSION\\\"" >> $CONFIG
-        echo 'LDLIBS += ' $LIBBPF_LDLIBS >> $CONFIG
+        printf '%s\n' "HAVE_LIBBPF:=y" >> $CONFIG
+        printf '%s\n' "CFLAGS += -DHAVE_LIBBPF $LIBBPF_CFLAGS" >> $CONFIG
+        printf '%s\n' "CFLAGS += -DLIBBPF_VERSION=\\\"$LIBBPF_VERSION\\\"" >> $CONFIG
+        printf '%s\n' "LDLIBS += $LIBBPF_LDLIBS" >> $CONFIG
 
         if [ -z "$LIBBPF_DIR" ]; then
-            echo "CFLAGS += -DLIBBPF_DYNAMIC" >> $CONFIG
+            printf '%s\n' "CFLAGS += -DLIBBPF_DYNAMIC" >> $CONFIG
         fi
     fi
 
     # bpf_program__title() is deprecated since libbpf 0.2.0, use
     # bpf_program__section_name() instead if we support
     if have_libbpf_sec_name; then
-        echo "HAVE_LIBBPF_SECTION_NAME:=y" >> $CONFIG
-        echo 'CFLAGS += -DHAVE_LIBBPF_SECTION_NAME ' >> $CONFIG
+        printf '%s\n' "HAVE_LIBBPF_SECTION_NAME:=y" >> $CONFIG
+        printf '%s\n' 'CFLAGS += -DHAVE_LIBBPF_SECTION_NAME ' >> $CONFIG
     fi
 
-    echo "yes"
-    echo "	libbpf version $LIBBPF_VERSION"
+    printf '%s\n' "yes"
+    printf '%s\n' "	libbpf version $LIBBPF_VERSION"
 }
 
 check_selinux()
 # SELinux is a compile time option in the ss utility
 {
 	if ${PKG_CONFIG} libselinux --exists; then
-		echo "HAVE_SELINUX:=y" >>$CONFIG
-		echo "yes"
+		printf '%s\n' "HAVE_SELINUX:=y" >>$CONFIG
+		printf '%s\n' "yes"
 
-		echo 'LDLIBS +=' "$(${PKG_CONFIG} --libs libselinux)" >>$CONFIG
-		echo 'CFLAGS += -DHAVE_SELINUX' "$(${PKG_CONFIG} --cflags libselinux)" >>$CONFIG
+		printf '%s\n' "LDLIBS +=$(${PKG_CONFIG} --libs libselinux)" >>$CONFIG
+		printf '%s\n' "CFLAGS += -DHAVE_SELINUX $(${PKG_CONFIG} --cflags libselinux)" >>$CONFIG
 	else
-		echo "no"
+		printf '%s\n' "no"
 	fi
 }
 
 check_tirpc()
 {
 	if ${PKG_CONFIG} libtirpc --exists; then
-		echo "HAVE_RPC:=y" >>$CONFIG
-		echo "yes"
+		printf '%s\n' "HAVE_RPC:=y" >>$CONFIG
+		printf '%s\n' "yes"
 
-		echo 'LDLIBS +=' "$(${PKG_CONFIG} --libs libtirpc)" >>$CONFIG
-		echo 'CFLAGS += -DHAVE_RPC' "$(${PKG_CONFIG} --cflags libtirpc)" >>$CONFIG
+		printf '%s\n' "LDLIBS += $(${PKG_CONFIG} --libs libtirpc)" >>$CONFIG
+		printf '%s\n' "CFLAGS += -DHAVE_RPC $(${PKG_CONFIG} --cflags libtirpc)" >>$CONFIG
 	else
-		echo "no"
+		printf '%s\n' "no"
 	fi
 }
 
 check_mnl()
 {
 	if ${PKG_CONFIG} libmnl --exists; then
-		echo "HAVE_MNL:=y" >>$CONFIG
-		echo "yes"
+		printf '%s\n' "HAVE_MNL:=y" >>$CONFIG
+		printf '%s\n' "yes"
 
-		echo 'CFLAGS += -DHAVE_LIBMNL' "$(${PKG_CONFIG} libmnl --cflags)" >>$CONFIG
-		echo 'LDLIBS +=' "$(${PKG_CONFIG} libmnl --libs)" >> $CONFIG
+		printf '%s\n' "CFLAGS += -DHAVE_LIBMNL $(${PKG_CONFIG} libmnl --cflags)" >>$CONFIG
+		printf '%s\n' "LDLIBS += $(${PKG_CONFIG} libmnl --libs)" >> $CONFIG
 	else
-		echo "no"
+		printf '%s\n' "no"
 	fi
 }
 
@@ -434,10 +435,10 @@ int main(int argc, char **argv) {
 }
 EOF
     if $CC -I$INCLUDE -o $TMPDIR/dbtest $TMPDIR/dbtest.c -ldb >/dev/null 2>&1; then
-	echo "HAVE_BERKELEY_DB:=y" >>$CONFIG
-	echo "yes"
+	printf '%s\n' "HAVE_BERKELEY_DB:=y" >>$CONFIG
+	printf '%s\n' "yes"
     else
-	echo "no"
+	printf '%s\n' "no"
     fi
     rm -f $TMPDIR/dbtest.c $TMPDIR/dbtest
 }
@@ -453,15 +454,15 @@ int main(int argc, char **argv) {
 }
 EOF
     if $CC -I$INCLUDE -o $TMPDIR/strtest $TMPDIR/strtest.c >/dev/null 2>&1; then
-	echo "no"
+	printf '%s\n' "no"
     else
 	if ${PKG_CONFIG} libbsd --exists; then
-		echo 'CFLAGS += -DHAVE_LIBBSD' "$(${PKG_CONFIG} libbsd --cflags)" >>$CONFIG
-		echo 'LDLIBS +=' "$(${PKG_CONFIG} libbsd --libs)" >> $CONFIG
-		echo "no"
+		printf '%s\n' "CFLAGS += -DHAVE_LIBBSD $(${PKG_CONFIG} libbsd --cflags)" >>$CONFIG
+		printf '%s\n' "LDLIBS += $(${PKG_CONFIG} libbsd --libs)" >> $CONFIG
+		printf '%s\n' "no"
 	else
-		echo 'CFLAGS += -DNEED_STRLCPY' >>$CONFIG
-		echo "yes"
+		printf '%s\n' 'CFLAGS += -DNEED_STRLCPY' >>$CONFIG
+		printf '%s\n' "yes"
 	fi
     fi
     rm -f $TMPDIR/strtest.c $TMPDIR/strtest
@@ -470,13 +471,13 @@ EOF
 check_cap()
 {
 	if ${PKG_CONFIG} libcap --exists; then
-		echo "HAVE_CAP:=y" >>$CONFIG
-		echo "yes"
+		printf '%s\n' "HAVE_CAP:=y" >>$CONFIG
+		printf '%s\n' "yes"
 
-		echo 'CFLAGS += -DHAVE_LIBCAP' "$(${PKG_CONFIG} libcap --cflags)" >>$CONFIG
-		echo 'LDLIBS +=' "$(${PKG_CONFIG} libcap --libs)" >> $CONFIG
+		printf '%s\n' "CFLAGS += -DHAVE_LIBCAP $(${PKG_CONFIG} libcap --cflags)" >>$CONFIG
+		printf '%s\n' "LDLIBS += $(${PKG_CONFIG} libcap --libs)" >> $CONFIG
 	else
-		echo "no"
+		printf '%s\n' "no"
 	fi
 }
 
@@ -484,16 +485,16 @@ check_color()
 {
 	case "$COLOR" in
 		never)
-			echo 'CONF_COLOR:=COLOR_OPT_NEVER' >> $CONFIG
-			echo 'never'
+			printf '%s\n' 'CONF_COLOR:=COLOR_OPT_NEVER' >> $CONFIG
+			printf '%s\n' 'never'
 			;;
 		auto)
-			echo 'CONF_COLOR:=COLOR_OPT_AUTO' >> $CONFIG
-			echo 'auto'
+			printf '%s\n' 'CONF_COLOR:=COLOR_OPT_AUTO' >> $CONFIG
+			printf '%s\n' 'auto'
 			;;
 		always)
-			echo 'CONF_COLOR:=COLOR_OPT_ALWAYS' >> $CONFIG
-			echo 'always'
+			printf '%s\n' 'CONF_COLOR:=COLOR_OPT_ALWAYS' >> $CONFIG
+			printf '%s\n' 'always'
 			;;
 	esac
 }
@@ -545,7 +546,7 @@ EOF
 }
 
 # Compat with the old INCLUDE path setting method.
-if [ $# -eq 1 ] && [ "$(echo $1 | cut -c 1)" != '-' ]; then
+if [ $# -eq 1 ] && [ "$(printf %s "$1" | cut -c 1)" != '-' ]; then
 	INCLUDE="$1"
 else
 	while [ "$#" -gt 0 ]; do
@@ -609,68 +610,70 @@ fi
 [ -z "$PREFIX" ] && usage 1
 [ -z "$LIBDIR" ] && usage 1
 
-echo "# Generated config based on" $INCLUDE >$CONFIG
+printf '%s\n' "# Generated config based on $INCLUDE" >$CONFIG
 quiet_config >> $CONFIG
 
 check_toolchain
 
-echo "TC schedulers"
+printf '%s\n' "TC schedulers"
 
-echo -n " ATM	"
+printf %s " ATM	"
 check_atm
 
 check_xtables
 if ! grep -q TC_CONFIG_NO_XT $CONFIG; then
-	echo -n " IPT	"
+	printf %s " IPT	"
 	check_xt
 	check_xt_old
 	check_xt_old_internal_h
 	check_ipt
 
-	echo -n " IPSET  "
+	printf %s " IPSET  "
 	check_ipset
 fi
 
-echo
+printf '\n'
 check_lib_dir
 if ! grep -q TC_CONFIG_NO_XT $CONFIG; then
-	echo -n "iptables modules directory: "
+	printf %s "iptables modules directory: "
 	check_ipt_lib_dir
 fi
 
-echo -n "libc has setns: "
+printf %s "libc has setns: "
 check_setns
 
-echo -n "libc has name_to_handle_at: "
+printf %s "libc has name_to_handle_at: "
 check_name_to_handle_at
 
-echo -n "SELinux support: "
+printf %s "SELinux support: "
 check_selinux
 
-echo -n "libtirpc support: "
+printf %s "libtirpc support: "
 check_tirpc
 
-echo -n "libbpf support: "
+printf %s "libbpf support: "
 check_libbpf
 
-echo -n "ELF support: "
+printf %s "ELF support: "
 check_elf
 
-echo -n "libmnl support: "
+printf %s "libmnl support: "
 check_mnl
 
-echo -n "Berkeley DB: "
+printf %s "Berkeley DB: "
 check_berkeley_db
 
-echo -n "need for strlcpy: "
+printf %s "need for strlcpy: "
 check_strlcpy
 
-echo -n "libcap support: "
+printf %s "libcap support: "
 check_cap
 
-echo -n "color output: "
+printf %s "color output: "
 check_color
 
-echo >> $CONFIG
-echo "%.o: %.c" >> $CONFIG
-echo '	$(QUIET_CC)$(CC) $(CFLAGS) $(EXTRA_CFLAGS) $(CPPFLAGS) -c -o $@ $<' >> $CONFIG
+{
+	printf '\n'
+	printf '%s\n' "%.o: %.c"
+	printf '%s\n' '	$(QUIET_CC)$(CC) $(CFLAGS) $(EXTRA_CFLAGS) $(CPPFLAGS) -c -o $@ $<'
+} >> $CONFIG
-- 
2.41.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH iproute2 1/2] configure: avoid un-recommended command substitution form
  2023-12-18  3:30 [PATCH iproute2 1/2] configure: avoid un-recommended command substitution form Eli Schwartz
  2023-12-18  3:30 ` [PATCH iproute2 2/2] configure: use the portable printf to suppress newlines in messages Eli Schwartz
@ 2023-12-28  0:46 ` Stephen Hemminger
  2023-12-28  3:57   ` Eli Schwartz
  1 sibling, 1 reply; 11+ messages in thread
From: Stephen Hemminger @ 2023-12-28  0:46 UTC (permalink / raw)
  To: Eli Schwartz; +Cc: netdev

On Sun, 17 Dec 2023 22:30:52 -0500
Eli Schwartz <eschwartz93@gmail.com> wrote:

> The use of backticks to surround commands instead of "$(cmd)" is a
> legacy of the oldest pre-POSIX shells. It is confusing, unreliable, and
> hard to read. Its use is not recommended in new programs.
> 
> See: http://mywiki.wooledge.org/BashFAQ/082
> ---

This is needless churn, it works now, and bash is never going
to drop the syntax.

Plus, the patch is missing signed-off-by.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH iproute2 2/2] configure: use the portable printf to suppress newlines in messages
  2023-12-18  3:30 ` [PATCH iproute2 2/2] configure: use the portable printf to suppress newlines in messages Eli Schwartz
@ 2023-12-28  0:46   ` Stephen Hemminger
  2023-12-28  3:53     ` Eli Schwartz
  2023-12-29  6:00     ` [PATCH 1/2] configure: avoid un-recommended command substitution form Eli Schwartz
  0 siblings, 2 replies; 11+ messages in thread
From: Stephen Hemminger @ 2023-12-28  0:46 UTC (permalink / raw)
  To: Eli Schwartz; +Cc: netdev

On Sun, 17 Dec 2023 22:30:53 -0500
Eli Schwartz <eschwartz93@gmail.com> wrote:

> Per https://pubs.opengroup.org/onlinepubs/9699919799/utilities/echo.html
> the "echo" utility is un-recommended and its behavior is non-portable
> and unpredictable. It *should* be marked as obsolescent, but was not,
> due solely "because of its extremely widespread use in historical
> applications".
> 
> POSIX doesn't require the -n option, and although its behavior is
> reliable in `#!/bin/bash` scripts, this configure script uses
> `#!/bin/sh` and cannot rely on echo -n.
> 
> The use of printf even without newline suppression or backslash
> character sequences is nicer for consistency, since there are a variety
> of ways it can go wrong with echo including "echoing the value of a
> shell or environment variable".
> 
> See:
> https://pubs.opengroup.org/onlinepubs/9699919799/utilities/echo.html
> https://cfajohnson.com/shell/cus-faq.html#Q0b
> ---

This is needless churn, it works now, and bash is never going
to remove the echo command. The script only has to work on Linux.

Plus, the patch is missing signed-off-by.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH iproute2 2/2] configure: use the portable printf to suppress newlines in messages
  2023-12-28  0:46   ` Stephen Hemminger
@ 2023-12-28  3:53     ` Eli Schwartz
  2023-12-29  6:00     ` [PATCH 1/2] configure: avoid un-recommended command substitution form Eli Schwartz
  1 sibling, 0 replies; 11+ messages in thread
From: Eli Schwartz @ 2023-12-28  3:53 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: netdev

On 12/27/23 7:46 PM, Stephen Hemminger wrote:
> On Sun, 17 Dec 2023 22:30:53 -0500
> Eli Schwartz <eschwartz93@gmail.com> wrote:
> 
>> Per https://pubs.opengroup.org/onlinepubs/9699919799/utilities/echo.html
>> the "echo" utility is un-recommended and its behavior is non-portable
>> and unpredictable. It *should* be marked as obsolescent, but was not,
>> due solely "because of its extremely widespread use in historical
>> applications".
>>
>> POSIX doesn't require the -n option, and although its behavior is
>> reliable in `#!/bin/bash` scripts, this configure script uses
>> `#!/bin/sh` and cannot rely on echo -n.
>>
>> The use of printf even without newline suppression or backslash
>> character sequences is nicer for consistency, since there are a variety
>> of ways it can go wrong with echo including "echoing the value of a
>> shell or environment variable".
>>
>> See:
>> https://pubs.opengroup.org/onlinepubs/9699919799/utilities/echo.html
>> https://cfajohnson.com/shell/cus-faq.html#Q0b
>> ---
> 
> This is needless churn, it works now, and bash is never going
> to remove the echo command. The script only has to work on Linux.


I think you've misunderstood something. It does not work now, because of
the reason stated in the patch message.

Whether bash does or does not remove the echo command is irrelevant.
Bash is NOT the only linux /bin/sh shell, and "it only has to work on
Linux" is NOT a valid reason to write code that is designed to work with
/bin/bash, but uses a shebang of /bin/sh.

Would you prefer to change the shebang to #!/bin/bash instead?


> Plus, the patch is missing signed-off-by.


I'm more than happy to send in an updated patch. :)


-- 
Eli Schwartz


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH iproute2 1/2] configure: avoid un-recommended command substitution form
  2023-12-28  0:46 ` [PATCH iproute2 1/2] configure: avoid un-recommended command substitution form Stephen Hemminger
@ 2023-12-28  3:57   ` Eli Schwartz
  2023-12-29 19:36     ` Stephen Hemminger
  0 siblings, 1 reply; 11+ messages in thread
From: Eli Schwartz @ 2023-12-28  3:57 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: netdev

On 12/27/23 7:46 PM, Stephen Hemminger wrote:
> On Sun, 17 Dec 2023 22:30:52 -0500
> Eli Schwartz <eschwartz93@gmail.com> wrote:
> 
>> The use of backticks to surround commands instead of "$(cmd)" is a
>> legacy of the oldest pre-POSIX shells. It is confusing, unreliable, and
>> hard to read. Its use is not recommended in new programs.
>>
>> See: http://mywiki.wooledge.org/BashFAQ/082
>> ---
> 
> This is needless churn, it works now, and bash is never going
> to drop the syntax.


Per the patch message, the reason to avoid the syntax is because it is
confusing, unreliable, and hard to read.

It was deprecated for good reason, and those reasons are relevant to
people writing shell scripts! Regardless of whether it is removed, it
has several very sharp edges and the modern alternative was designed
specifically because the legacy syntax is bad to use *even in bash*.

(bash has nothing to do with it. But also, again, this is not about bash
because the configure script shebang is *not* /bin/bash.)


-- 
Eli Schwartz


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 1/2] configure: avoid un-recommended command substitution form
  2023-12-28  0:46   ` Stephen Hemminger
  2023-12-28  3:53     ` Eli Schwartz
@ 2023-12-29  6:00     ` Eli Schwartz
  2023-12-29  6:00       ` [PATCH 2/2] configure: use the portable printf to suppress newlines in messages Eli Schwartz
  1 sibling, 1 reply; 11+ messages in thread
From: Eli Schwartz @ 2023-12-29  6:00 UTC (permalink / raw)
  To: netdev; +Cc: stephen

The use of backticks to surround commands instead of "$(cmd)" is a
legacy of the oldest pre-POSIX shells. It is confusing, unreliable, and
hard to read. Its use is not recommended in new programs.

See: http://mywiki.wooledge.org/BashFAQ/082
Signed-off-by: Eli Schwartz <eschwartz93@gmail.com>
---
 configure | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/configure b/configure
index 488c5f88..158e76e1 100755
--- a/configure
+++ b/configure
@@ -250,8 +250,8 @@ check_elf()
 	echo "HAVE_ELF:=y" >>$CONFIG
 	echo "yes"
 
-	echo 'CFLAGS += -DHAVE_ELF' `${PKG_CONFIG} libelf --cflags` >> $CONFIG
-	echo 'LDLIBS += ' `${PKG_CONFIG} libelf --libs` >>$CONFIG
+	echo 'CFLAGS += -DHAVE_ELF' "$(${PKG_CONFIG} libelf --cflags)" >> $CONFIG
+	echo 'LDLIBS += ' "$(${PKG_CONFIG} libelf --libs)" >>$CONFIG
     else
 	echo "no"
     fi
@@ -369,8 +369,8 @@ check_selinux()
 		echo "HAVE_SELINUX:=y" >>$CONFIG
 		echo "yes"
 
-		echo 'LDLIBS +=' `${PKG_CONFIG} --libs libselinux` >>$CONFIG
-		echo 'CFLAGS += -DHAVE_SELINUX' `${PKG_CONFIG} --cflags libselinux` >>$CONFIG
+		echo 'LDLIBS +=' "$(${PKG_CONFIG} --libs libselinux)" >>$CONFIG
+		echo 'CFLAGS += -DHAVE_SELINUX' "$(${PKG_CONFIG} --cflags libselinux)" >>$CONFIG
 	else
 		echo "no"
 	fi
@@ -382,8 +382,8 @@ check_tirpc()
 		echo "HAVE_RPC:=y" >>$CONFIG
 		echo "yes"
 
-		echo 'LDLIBS +=' `${PKG_CONFIG} --libs libtirpc` >>$CONFIG
-		echo 'CFLAGS += -DHAVE_RPC' `${PKG_CONFIG} --cflags libtirpc` >>$CONFIG
+		echo 'LDLIBS +=' "$(${PKG_CONFIG} --libs libtirpc)" >>$CONFIG
+		echo 'CFLAGS += -DHAVE_RPC' "$(${PKG_CONFIG} --cflags libtirpc)" >>$CONFIG
 	else
 		echo "no"
 	fi
@@ -395,8 +395,8 @@ check_mnl()
 		echo "HAVE_MNL:=y" >>$CONFIG
 		echo "yes"
 
-		echo 'CFLAGS += -DHAVE_LIBMNL' `${PKG_CONFIG} libmnl --cflags` >>$CONFIG
-		echo 'LDLIBS +=' `${PKG_CONFIG} libmnl --libs` >> $CONFIG
+		echo 'CFLAGS += -DHAVE_LIBMNL' "$(${PKG_CONFIG} libmnl --cflags)" >>$CONFIG
+		echo 'LDLIBS +=' "$(${PKG_CONFIG} libmnl --libs)" >> $CONFIG
 	else
 		echo "no"
 	fi
@@ -436,8 +436,8 @@ EOF
 	echo "no"
     else
 	if ${PKG_CONFIG} libbsd --exists; then
-		echo 'CFLAGS += -DHAVE_LIBBSD' `${PKG_CONFIG} libbsd --cflags` >>$CONFIG
-		echo 'LDLIBS +=' `${PKG_CONFIG} libbsd --libs` >> $CONFIG
+		echo 'CFLAGS += -DHAVE_LIBBSD' "$(${PKG_CONFIG} libbsd --cflags)" >>$CONFIG
+		echo 'LDLIBS +=' "$(${PKG_CONFIG} libbsd --libs)" >> $CONFIG
 		echo "no"
 	else
 		echo 'CFLAGS += -DNEED_STRLCPY' >>$CONFIG
@@ -453,8 +453,8 @@ check_cap()
 		echo "HAVE_CAP:=y" >>$CONFIG
 		echo "yes"
 
-		echo 'CFLAGS += -DHAVE_LIBCAP' `${PKG_CONFIG} libcap --cflags` >>$CONFIG
-		echo 'LDLIBS +=' `${PKG_CONFIG} libcap --libs` >> $CONFIG
+		echo 'CFLAGS += -DHAVE_LIBCAP' "$(${PKG_CONFIG} libcap --cflags)" >>$CONFIG
+		echo 'LDLIBS +=' "$(${PKG_CONFIG} libcap --libs)" >> $CONFIG
 	else
 		echo "no"
 	fi
-- 
2.41.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 2/2] configure: use the portable printf to suppress newlines in messages
  2023-12-29  6:00     ` [PATCH 1/2] configure: avoid un-recommended command substitution form Eli Schwartz
@ 2023-12-29  6:00       ` Eli Schwartz
  2024-01-01 19:03         ` Stephen Hemminger
  0 siblings, 1 reply; 11+ messages in thread
From: Eli Schwartz @ 2023-12-29  6:00 UTC (permalink / raw)
  To: netdev; +Cc: stephen

Per https://pubs.opengroup.org/onlinepubs/9699919799/utilities/echo.html
the "echo" utility is un-recommended and its behavior is non-portable
and unpredictable. It *should* be marked as obsolescent, but was not,
due solely "because of its extremely widespread use in historical
applications".

POSIX doesn't require the -n option, and although its behavior is
reliable in `#!/bin/bash` scripts, this configure script uses
`#!/bin/sh` and cannot rely on echo -n.

The use of printf even without newline suppression or backslash
character sequences is nicer for consistency, since there are a variety
of ways it can go wrong with echo including "echoing the value of a
shell or environment variable".

See: https://pubs.opengroup.org/onlinepubs/9699919799/utilities/echo.html
See: https://cfajohnson.com/shell/cus-faq.html#Q0b
Signed-off-by: Eli Schwartz <eschwartz93@gmail.com>
---
 configure | 215 +++++++++++++++++++++++++++---------------------------
 1 file changed, 109 insertions(+), 106 deletions(-)

diff --git a/configure b/configure
index 158e76e1..a2753f43 100755
--- a/configure
+++ b/configure
@@ -20,16 +20,18 @@ check_toolchain()
     : ${AR=ar}
     : ${CC=gcc}
     : ${YACC=bison}
-    echo "PKG_CONFIG:=${PKG_CONFIG}" >>$CONFIG
-    echo "AR:=${AR}" >>$CONFIG
-    echo "CC:=${CC}" >>$CONFIG
-    echo "YACC:=${YACC}" >>$CONFIG
+    {
+        printf '%s\n' "PKG_CONFIG:=${PKG_CONFIG}"
+        printf '%s\n' "AR:=${AR}"
+        printf '%s\n' "CC:=${CC}"
+        printf '%s\n' "YACC:=${YACC}"
+    } >>$CONFIG
 }
 
 check_xtables()
 {
 	if ! ${PKG_CONFIG} xtables --exists; then
-		echo "TC_CONFIG_NO_XT:=y" >>$CONFIG
+		printf '%s\n' "TC_CONFIG_NO_XT:=y" >>$CONFIG
 	fi
 }
 
@@ -57,8 +59,8 @@ EOF
 
     if $CC -I$INCLUDE $IPTC -o $TMPDIR/ipttest $TMPDIR/ipttest.c $IPTL \
 	$(${PKG_CONFIG} xtables --cflags --libs) -ldl >/dev/null 2>&1; then
-	echo "TC_CONFIG_XT:=y" >>$CONFIG
-	echo "using xtables"
+	printf '%s\n' "TC_CONFIG_XT:=y" >>$CONFIG
+	printf '%s\n' "using xtables"
     fi
     rm -f $TMPDIR/ipttest.c $TMPDIR/ipttest
 }
@@ -91,8 +93,8 @@ int main(int argc, char **argv) {
 EOF
 
     if $CC -I$INCLUDE $IPTC -o $TMPDIR/ipttest $TMPDIR/ipttest.c $IPTL -ldl >/dev/null 2>&1; then
-	echo "TC_CONFIG_XT_OLD:=y" >>$CONFIG
-	echo "using old xtables (no need for xt-internal.h)"
+	printf '%s\n' "TC_CONFIG_XT_OLD:=y" >>$CONFIG
+	printf '%s\n' "using old xtables (no need for xt-internal.h)"
     fi
     rm -f $TMPDIR/ipttest.c $TMPDIR/ipttest
 }
@@ -125,25 +127,24 @@ int main(int argc, char **argv) {
 
 EOF
 	if $CC -I$INCLUDE $IPTC -o $TMPDIR/ipttest $TMPDIR/ipttest.c $IPTL -ldl >/dev/null 2>&1; then
-	    echo "using old xtables with xt-internal.h"
-	    echo "TC_CONFIG_XT_OLD_H:=y" >>$CONFIG
+	    printf '%s\n' "using old xtables with xt-internal.h"
+	    printf '%s\n' "TC_CONFIG_XT_OLD_H:=y" >>$CONFIG
 	fi
 	rm -f $TMPDIR/ipttest.c $TMPDIR/ipttest
 }
 
 check_lib_dir()
 {
-	LIBDIR=$(echo $LIBDIR | sed "s|\${prefix}|$PREFIX|")
+	LIBDIR=$(printf '%s' "$LIBDIR" | sed "s|\${prefix}|$PREFIX|")
 
-	echo -n "lib directory: "
-	echo "$LIBDIR"
-	echo "LIBDIR:=$LIBDIR" >> $CONFIG
+	printf '%s\n' "lib directory: $LIBDIR"
+	printf '%s\n' "LIBDIR:=$LIBDIR" >> $CONFIG
 }
 
 check_ipt()
 {
 	if ! grep TC_CONFIG_XT $CONFIG > /dev/null; then
-		echo "using iptables"
+		printf '%s\n' "using iptables"
 	fi
 }
 
@@ -151,8 +152,8 @@ check_ipt_lib_dir()
 {
 	IPT_LIB_DIR=$(${PKG_CONFIG} --variable=xtlibdir xtables)
 	if [ -n "$IPT_LIB_DIR" ]; then
-		echo $IPT_LIB_DIR
-		echo "IPT_LIB_DIR:=$IPT_LIB_DIR" >> $CONFIG
+		printf '%s\n' "$IPT_LIB_DIR"
+		printf '%s\n' "IPT_LIB_DIR:=$IPT_LIB_DIR" >> $CONFIG
 		return
 	fi
 
@@ -160,13 +161,13 @@ check_ipt_lib_dir()
 		for file in "xtables" "iptables"; do
 			file="$dir/$file/lib*t_*so"
 			if [ -f $file ]; then
-				echo ${file%/*}
-				echo "IPT_LIB_DIR:=${file%/*}" >> $CONFIG
+				printf '%s\n' "${file%/*}"
+				printf '%s\n' "IPT_LIB_DIR:=${file%/*}" >> $CONFIG
 				return
 			fi
 		done
 	done
-	echo "not found!"
+	printf '%s\n' "not found!"
 }
 
 check_setns()
@@ -181,11 +182,11 @@ int main(int argc, char **argv)
 }
 EOF
     if $CC -I$INCLUDE -o $TMPDIR/setnstest $TMPDIR/setnstest.c >/dev/null 2>&1; then
-	echo "IP_CONFIG_SETNS:=y" >>$CONFIG
-	echo "yes"
-	echo "CFLAGS += -DHAVE_SETNS" >>$CONFIG
+	printf '%s\n' "IP_CONFIG_SETNS:=y" >>$CONFIG
+	printf '%s\n' "yes"
+	printf '%s\n' "CFLAGS += -DHAVE_SETNS" >>$CONFIG
     else
-	echo "no"
+	printf '%s\n' "no"
     fi
     rm -f $TMPDIR/setnstest.c $TMPDIR/setnstest
 }
@@ -206,10 +207,10 @@ int main(int argc, char **argv)
 }
 EOF
     if $CC -I$INCLUDE -o $TMPDIR/name_to_handle_at_test $TMPDIR/name_to_handle_at_test.c >/dev/null 2>&1; then
-        echo "yes"
-        echo "CFLAGS += -DHAVE_HANDLE_AT" >>$CONFIG
+        printf '%s\n' "yes"
+        printf '%s\n' "CFLAGS += -DHAVE_HANDLE_AT" >>$CONFIG
     else
-        echo "no"
+        printf '%s\n' "no"
     fi
     rm -f $TMPDIR/name_to_handle_at_test.c $TMPDIR/name_to_handle_at_test
 }
@@ -236,10 +237,10 @@ int main(void)
 EOF
 
     if $CC -I$INCLUDE -o $TMPDIR/ipsettest $TMPDIR/ipsettest.c >/dev/null 2>&1; then
-	echo "TC_CONFIG_IPSET:=y" >>$CONFIG
-	echo "yes"
+	printf '%s\n' "TC_CONFIG_IPSET:=y" >>$CONFIG
+	printf '%s\n' "yes"
     else
-	echo "no"
+	printf '%s\n' "no"
     fi
     rm -f $TMPDIR/ipsettest.c $TMPDIR/ipsettest
 }
@@ -247,13 +248,13 @@ EOF
 check_elf()
 {
     if ${PKG_CONFIG} libelf --exists; then
-	echo "HAVE_ELF:=y" >>$CONFIG
-	echo "yes"
+	printf '%s\n' "HAVE_ELF:=y" >>$CONFIG
+	printf '%s\n' "yes"
 
-	echo 'CFLAGS += -DHAVE_ELF' "$(${PKG_CONFIG} libelf --cflags)" >> $CONFIG
-	echo 'LDLIBS += ' "$(${PKG_CONFIG} libelf --libs)" >>$CONFIG
+	printf '%s\n' "CFLAGS += -DHAVE_ELF $(${PKG_CONFIG} libelf --cflags)" >> $CONFIG
+	printf '%s\n' "LDLIBS += $(${PKG_CONFIG} libelf --libs)" >>$CONFIG
     else
-	echo "no"
+	printf '%s\n' "no"
     fi
 }
 
@@ -300,7 +301,7 @@ check_force_libbpf_on()
     # if set LIBBPF_FORCE=on but no libbpf support, just exist the config
     # process to make sure we don't build without libbpf.
     if [ "$LIBBPF_FORCE" = on ]; then
-        echo "	LIBBPF_FORCE=on set, but couldn't find a usable libbpf"
+        printf '%s\n' "	LIBBPF_FORCE=on set, but couldn't find a usable libbpf"
         exit 1
     fi
 }
@@ -309,12 +310,12 @@ check_libbpf()
 {
     # if set LIBBPF_FORCE=off, disable libbpf entirely
     if [ "$LIBBPF_FORCE" = off ]; then
-        echo "no"
+        printf '%s\n' "no"
         return
     fi
 
     if ! ${PKG_CONFIG} libbpf --exists && [ -z "$LIBBPF_DIR" ] ; then
-        echo "no"
+        printf '%s\n' "no"
         check_force_libbpf_on
         return
     fi
@@ -336,69 +337,69 @@ check_libbpf()
     fi
 
     if ! have_libbpf_basic; then
-        echo "no"
-        echo "	libbpf version $LIBBPF_VERSION is too low, please update it to at least 0.1.0"
+        printf '%s\n' "no"
+        printf '%s\n' "	libbpf version $LIBBPF_VERSION is too low, please update it to at least 0.1.0"
         check_force_libbpf_on
         return
     else
-        echo "HAVE_LIBBPF:=y" >> $CONFIG
-        echo 'CFLAGS += -DHAVE_LIBBPF ' $LIBBPF_CFLAGS >> $CONFIG
-        echo "CFLAGS += -DLIBBPF_VERSION=\\\"$LIBBPF_VERSION\\\"" >> $CONFIG
-        echo 'LDLIBS += ' $LIBBPF_LDLIBS >> $CONFIG
+        printf '%s\n' "HAVE_LIBBPF:=y" >> $CONFIG
+        printf '%s\n' "CFLAGS += -DHAVE_LIBBPF $LIBBPF_CFLAGS" >> $CONFIG
+        printf '%s\n' "CFLAGS += -DLIBBPF_VERSION=\\\"$LIBBPF_VERSION\\\"" >> $CONFIG
+        printf '%s\n' "LDLIBS += $LIBBPF_LDLIBS" >> $CONFIG
 
         if [ -z "$LIBBPF_DIR" ]; then
-            echo "CFLAGS += -DLIBBPF_DYNAMIC" >> $CONFIG
+            printf '%s\n' "CFLAGS += -DLIBBPF_DYNAMIC" >> $CONFIG
         fi
     fi
 
     # bpf_program__title() is deprecated since libbpf 0.2.0, use
     # bpf_program__section_name() instead if we support
     if have_libbpf_sec_name; then
-        echo "HAVE_LIBBPF_SECTION_NAME:=y" >> $CONFIG
-        echo 'CFLAGS += -DHAVE_LIBBPF_SECTION_NAME ' >> $CONFIG
+        printf '%s\n' "HAVE_LIBBPF_SECTION_NAME:=y" >> $CONFIG
+        printf '%s\n' 'CFLAGS += -DHAVE_LIBBPF_SECTION_NAME ' >> $CONFIG
     fi
 
-    echo "yes"
-    echo "	libbpf version $LIBBPF_VERSION"
+    printf '%s\n' "yes"
+    printf '%s\n' "	libbpf version $LIBBPF_VERSION"
 }
 
 check_selinux()
 # SELinux is a compile time option in the ss utility
 {
 	if ${PKG_CONFIG} libselinux --exists; then
-		echo "HAVE_SELINUX:=y" >>$CONFIG
-		echo "yes"
+		printf '%s\n' "HAVE_SELINUX:=y" >>$CONFIG
+		printf '%s\n' "yes"
 
-		echo 'LDLIBS +=' "$(${PKG_CONFIG} --libs libselinux)" >>$CONFIG
-		echo 'CFLAGS += -DHAVE_SELINUX' "$(${PKG_CONFIG} --cflags libselinux)" >>$CONFIG
+		printf '%s\n' "LDLIBS +=$(${PKG_CONFIG} --libs libselinux)" >>$CONFIG
+		printf '%s\n' "CFLAGS += -DHAVE_SELINUX $(${PKG_CONFIG} --cflags libselinux)" >>$CONFIG
 	else
-		echo "no"
+		printf '%s\n' "no"
 	fi
 }
 
 check_tirpc()
 {
 	if ${PKG_CONFIG} libtirpc --exists; then
-		echo "HAVE_RPC:=y" >>$CONFIG
-		echo "yes"
+		printf '%s\n' "HAVE_RPC:=y" >>$CONFIG
+		printf '%s\n' "yes"
 
-		echo 'LDLIBS +=' "$(${PKG_CONFIG} --libs libtirpc)" >>$CONFIG
-		echo 'CFLAGS += -DHAVE_RPC' "$(${PKG_CONFIG} --cflags libtirpc)" >>$CONFIG
+		printf '%s\n' "LDLIBS += $(${PKG_CONFIG} --libs libtirpc)" >>$CONFIG
+		printf '%s\n' "CFLAGS += -DHAVE_RPC $(${PKG_CONFIG} --cflags libtirpc)" >>$CONFIG
 	else
-		echo "no"
+		printf '%s\n' "no"
 	fi
 }
 
 check_mnl()
 {
 	if ${PKG_CONFIG} libmnl --exists; then
-		echo "HAVE_MNL:=y" >>$CONFIG
-		echo "yes"
+		printf '%s\n' "HAVE_MNL:=y" >>$CONFIG
+		printf '%s\n' "yes"
 
-		echo 'CFLAGS += -DHAVE_LIBMNL' "$(${PKG_CONFIG} libmnl --cflags)" >>$CONFIG
-		echo 'LDLIBS +=' "$(${PKG_CONFIG} libmnl --libs)" >> $CONFIG
+		printf '%s\n' "CFLAGS += -DHAVE_LIBMNL $(${PKG_CONFIG} libmnl --cflags)" >>$CONFIG
+		printf '%s\n' "LDLIBS += $(${PKG_CONFIG} libmnl --libs)" >> $CONFIG
 	else
-		echo "no"
+		printf '%s\n' "no"
 	fi
 }
 
@@ -414,10 +415,10 @@ int main(int argc, char **argv) {
 }
 EOF
     if $CC -I$INCLUDE -o $TMPDIR/dbtest $TMPDIR/dbtest.c -ldb >/dev/null 2>&1; then
-	echo "HAVE_BERKELEY_DB:=y" >>$CONFIG
-	echo "yes"
+	printf '%s\n' "HAVE_BERKELEY_DB:=y" >>$CONFIG
+	printf '%s\n' "yes"
     else
-	echo "no"
+	printf '%s\n' "no"
     fi
     rm -f $TMPDIR/dbtest.c $TMPDIR/dbtest
 }
@@ -433,15 +434,15 @@ int main(int argc, char **argv) {
 }
 EOF
     if $CC -I$INCLUDE -o $TMPDIR/strtest $TMPDIR/strtest.c >/dev/null 2>&1; then
-	echo "no"
+	printf '%s\n' "no"
     else
 	if ${PKG_CONFIG} libbsd --exists; then
-		echo 'CFLAGS += -DHAVE_LIBBSD' "$(${PKG_CONFIG} libbsd --cflags)" >>$CONFIG
-		echo 'LDLIBS +=' "$(${PKG_CONFIG} libbsd --libs)" >> $CONFIG
-		echo "no"
+		printf '%s\n' "CFLAGS += -DHAVE_LIBBSD $(${PKG_CONFIG} libbsd --cflags)" >>$CONFIG
+		printf '%s\n' "LDLIBS += $(${PKG_CONFIG} libbsd --libs)" >> $CONFIG
+		printf '%s\n' "no"
 	else
-		echo 'CFLAGS += -DNEED_STRLCPY' >>$CONFIG
-		echo "yes"
+		printf '%s\n' 'CFLAGS += -DNEED_STRLCPY' >>$CONFIG
+		printf '%s\n' "yes"
 	fi
     fi
     rm -f $TMPDIR/strtest.c $TMPDIR/strtest
@@ -450,13 +451,13 @@ EOF
 check_cap()
 {
 	if ${PKG_CONFIG} libcap --exists; then
-		echo "HAVE_CAP:=y" >>$CONFIG
-		echo "yes"
+		printf '%s\n' "HAVE_CAP:=y" >>$CONFIG
+		printf '%s\n' "yes"
 
-		echo 'CFLAGS += -DHAVE_LIBCAP' "$(${PKG_CONFIG} libcap --cflags)" >>$CONFIG
-		echo 'LDLIBS +=' "$(${PKG_CONFIG} libcap --libs)" >> $CONFIG
+		printf '%s\n' "CFLAGS += -DHAVE_LIBCAP $(${PKG_CONFIG} libcap --cflags)" >>$CONFIG
+		printf '%s\n' "LDLIBS += $(${PKG_CONFIG} libcap --libs)" >> $CONFIG
 	else
-		echo "no"
+		printf '%s\n' "no"
 	fi
 }
 
@@ -464,16 +465,16 @@ check_color()
 {
 	case "$COLOR" in
 		never)
-			echo 'CONF_COLOR:=COLOR_OPT_NEVER' >> $CONFIG
-			echo 'never'
+			printf '%s\n' 'CONF_COLOR:=COLOR_OPT_NEVER' >> $CONFIG
+			printf '%s\n' 'never'
 			;;
 		auto)
-			echo 'CONF_COLOR:=COLOR_OPT_AUTO' >> $CONFIG
-			echo 'auto'
+			printf '%s\n' 'CONF_COLOR:=COLOR_OPT_AUTO' >> $CONFIG
+			printf '%s\n' 'auto'
 			;;
 		always)
-			echo 'CONF_COLOR:=COLOR_OPT_ALWAYS' >> $CONFIG
-			echo 'always'
+			printf '%s\n' 'CONF_COLOR:=COLOR_OPT_ALWAYS' >> $CONFIG
+			printf '%s\n' 'always'
 			;;
 	esac
 }
@@ -525,7 +526,7 @@ EOF
 }
 
 # Compat with the old INCLUDE path setting method.
-if [ $# -eq 1 ] && [ "$(echo $1 | cut -c 1)" != '-' ]; then
+if [ $# -eq 1 ] && [ "$(printf %s "$1" | cut -c 1)" != '-' ]; then
 	INCLUDE="$1"
 else
 	while [ "$#" -gt 0 ]; do
@@ -589,65 +590,67 @@ fi
 [ -z "$PREFIX" ] && usage 1
 [ -z "$LIBDIR" ] && usage 1
 
-echo "# Generated config based on" $INCLUDE >$CONFIG
+printf '%s\n' "# Generated config based on $INCLUDE" >$CONFIG
 quiet_config >> $CONFIG
 
 check_toolchain
 
-echo "TC schedulers"
+printf '%s\n' "TC schedulers"
 
 check_xtables
 if ! grep -q TC_CONFIG_NO_XT $CONFIG; then
-	echo -n " IPT	"
+	printf %s " IPT	"
 	check_xt
 	check_xt_old
 	check_xt_old_internal_h
 	check_ipt
 
-	echo -n " IPSET  "
+	printf %s " IPSET  "
 	check_ipset
 fi
 
-echo
+printf '\n'
 check_lib_dir
 if ! grep -q TC_CONFIG_NO_XT $CONFIG; then
-	echo -n "iptables modules directory: "
+	printf %s "iptables modules directory: "
 	check_ipt_lib_dir
 fi
 
-echo -n "libc has setns: "
+printf %s "libc has setns: "
 check_setns
 
-echo -n "libc has name_to_handle_at: "
+printf %s "libc has name_to_handle_at: "
 check_name_to_handle_at
 
-echo -n "SELinux support: "
+printf %s "SELinux support: "
 check_selinux
 
-echo -n "libtirpc support: "
+printf %s "libtirpc support: "
 check_tirpc
 
-echo -n "libbpf support: "
+printf %s "libbpf support: "
 check_libbpf
 
-echo -n "ELF support: "
+printf %s "ELF support: "
 check_elf
 
-echo -n "libmnl support: "
+printf %s "libmnl support: "
 check_mnl
 
-echo -n "Berkeley DB: "
+printf %s "Berkeley DB: "
 check_berkeley_db
 
-echo -n "need for strlcpy: "
+printf %s "need for strlcpy: "
 check_strlcpy
 
-echo -n "libcap support: "
+printf %s "libcap support: "
 check_cap
 
-echo -n "color output: "
+printf %s "color output: "
 check_color
 
-echo >> $CONFIG
-echo "%.o: %.c" >> $CONFIG
-echo '	$(QUIET_CC)$(CC) $(CFLAGS) $(EXTRA_CFLAGS) $(CPPFLAGS) -c -o $@ $<' >> $CONFIG
+{
+	printf '\n'
+	printf '%s\n' "%.o: %.c"
+	printf '%s\n' '	$(QUIET_CC)$(CC) $(CFLAGS) $(EXTRA_CFLAGS) $(CPPFLAGS) -c -o $@ $<'
+} >> $CONFIG
-- 
2.41.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH iproute2 1/2] configure: avoid un-recommended command substitution form
  2023-12-28  3:57   ` Eli Schwartz
@ 2023-12-29 19:36     ` Stephen Hemminger
  2023-12-30  0:42       ` David Ahern
  0 siblings, 1 reply; 11+ messages in thread
From: Stephen Hemminger @ 2023-12-29 19:36 UTC (permalink / raw)
  To: Eli Schwartz, David Ahern; +Cc: netdev

On Wed, 27 Dec 2023 22:57:10 -0500
Eli Schwartz <eschwartz93@gmail.com> wrote:

> On 12/27/23 7:46 PM, Stephen Hemminger wrote:
> > On Sun, 17 Dec 2023 22:30:52 -0500
> > Eli Schwartz <eschwartz93@gmail.com> wrote:
> >   
> >> The use of backticks to surround commands instead of "$(cmd)" is a
> >> legacy of the oldest pre-POSIX shells. It is confusing, unreliable, and
> >> hard to read. Its use is not recommended in new programs.
> >>
> >> See: http://mywiki.wooledge.org/BashFAQ/082
> >> ---  
> > 
> > This is needless churn, it works now, and bash is never going
> > to drop the syntax.  
> 
> 
> Per the patch message, the reason to avoid the syntax is because it is
> confusing, unreliable, and hard to read.
> 
> It was deprecated for good reason, and those reasons are relevant to
> people writing shell scripts! Regardless of whether it is removed, it
> has several very sharp edges and the modern alternative was designed
> specifically because the legacy syntax is bad to use *even in bash*.
> 
> (bash has nothing to do with it. But also, again, this is not about bash
> because the configure script shebang is *not* /bin/bash.)

The existing configuration was built incrementally over time.
Mostly as a reaction to the issues with autoconf.

Perhaps it is time to consider updating iproute2 to a modern build
environment like meson that has better config support.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH iproute2 1/2] configure: avoid un-recommended command substitution form
  2023-12-29 19:36     ` Stephen Hemminger
@ 2023-12-30  0:42       ` David Ahern
  0 siblings, 0 replies; 11+ messages in thread
From: David Ahern @ 2023-12-30  0:42 UTC (permalink / raw)
  To: Stephen Hemminger, Eli Schwartz, David Ahern; +Cc: netdev

On 12/29/23 2:36 PM, Stephen Hemminger wrote:
> On Wed, 27 Dec 2023 22:57:10 -0500
> Eli Schwartz <eschwartz93@gmail.com> wrote:
> 
>> On 12/27/23 7:46 PM, Stephen Hemminger wrote:
>>> On Sun, 17 Dec 2023 22:30:52 -0500
>>> Eli Schwartz <eschwartz93@gmail.com> wrote:
>>>   
>>>> The use of backticks to surround commands instead of "$(cmd)" is a
>>>> legacy of the oldest pre-POSIX shells. It is confusing, unreliable, and
>>>> hard to read. Its use is not recommended in new programs.
>>>>
>>>> See: http://mywiki.wooledge.org/BashFAQ/082
>>>> ---  
>>>
>>> This is needless churn, it works now, and bash is never going
>>> to drop the syntax.  
>>
>>
>> Per the patch message, the reason to avoid the syntax is because it is
>> confusing, unreliable, and hard to read.
>>
>> It was deprecated for good reason, and those reasons are relevant to
>> people writing shell scripts! Regardless of whether it is removed, it
>> has several very sharp edges and the modern alternative was designed
>> specifically because the legacy syntax is bad to use *even in bash*.
>>
>> (bash has nothing to do with it. But also, again, this is not about bash
>> because the configure script shebang is *not* /bin/bash.)
> 
> The existing configuration was built incrementally over time.
> Mostly as a reaction to the issues with autoconf.
> 
> Perhaps it is time to consider updating iproute2 to a modern build
> environment like meson that has better config support.
> 


I do find $(...) much more readable than `...`; those little tick marks
can get lost on a screen with any dust for example.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/2] configure: use the portable printf to suppress newlines in messages
  2023-12-29  6:00       ` [PATCH 2/2] configure: use the portable printf to suppress newlines in messages Eli Schwartz
@ 2024-01-01 19:03         ` Stephen Hemminger
  0 siblings, 0 replies; 11+ messages in thread
From: Stephen Hemminger @ 2024-01-01 19:03 UTC (permalink / raw)
  To: Eli Schwartz; +Cc: netdev

On Fri, 29 Dec 2023 01:00:10 -0500
Eli Schwartz <eschwartz93@gmail.com> wrote:

> Per https://pubs.opengroup.org/onlinepubs/9699919799/utilities/echo.html
> the "echo" utility is un-recommended and its behavior is non-portable
> and unpredictable. It *should* be marked as obsolescent, but was not,
> due solely "because of its extremely widespread use in historical
> applications".
> 
> POSIX doesn't require the -n option, and although its behavior is
> reliable in `#!/bin/bash` scripts, this configure script uses
> `#!/bin/sh` and cannot rely on echo -n.
> 
> The use of printf even without newline suppression or backslash
> character sequences is nicer for consistency, since there are a variety
> of ways it can go wrong with echo including "echoing the value of a
> shell or environment variable".
> 
> See: https://pubs.opengroup.org/onlinepubs/9699919799/utilities/echo.html
> See: https://cfajohnson.com/shell/cus-faq.html#Q0b
> Signed-off-by: Eli Schwartz <eschwartz93@gmail.com>

This part is not necessary. Although echo is not portable, it is used
many places and not worth changing.

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2024-01-01 19:03 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-12-18  3:30 [PATCH iproute2 1/2] configure: avoid un-recommended command substitution form Eli Schwartz
2023-12-18  3:30 ` [PATCH iproute2 2/2] configure: use the portable printf to suppress newlines in messages Eli Schwartz
2023-12-28  0:46   ` Stephen Hemminger
2023-12-28  3:53     ` Eli Schwartz
2023-12-29  6:00     ` [PATCH 1/2] configure: avoid un-recommended command substitution form Eli Schwartz
2023-12-29  6:00       ` [PATCH 2/2] configure: use the portable printf to suppress newlines in messages Eli Schwartz
2024-01-01 19:03         ` Stephen Hemminger
2023-12-28  0:46 ` [PATCH iproute2 1/2] configure: avoid un-recommended command substitution form Stephen Hemminger
2023-12-28  3:57   ` Eli Schwartz
2023-12-29 19:36     ` Stephen Hemminger
2023-12-30  0:42       ` David Ahern

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).