* [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 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
* [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 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
* 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 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
* 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
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).