diff mbox series

[1/4] base-files/functions.sh: read grep output by line

Message ID 20200920053140.35255-1-rosenp@gmail.com
State Superseded
Headers show
Series [1/4] base-files/functions.sh: read grep output by line | expand

Commit Message

Rosen Penev Sept. 20, 2020, 5:31 a.m. UTC
From: https://github.com/koalaman/shellcheck/wiki/SC2013

For loops by default (subject to $IFS) read word by word.
Additionally, glob expansion will occur.

I believe the intended use case of these loops is to read lines.

Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 package/base-files/files/lib/functions.sh | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

Comments

Adrian Schmutzler Sept. 25, 2020, 4:19 p.m. UTC | #1
Hi,

> -----Original Message-----
> From: openwrt-devel [mailto:openwrt-devel-bounces@lists.openwrt.org]
> On Behalf Of Rosen Penev
> Sent: Sonntag, 20. September 2020 07:32
> To: openwrt-devel@lists.openwrt.org
> Subject: [PATCH 1/4] base-files/functions.sh: read grep output by line
> 
> From: https://github.com/koalaman/shellcheck/wiki/SC2013
> 
> For loops by default (subject to $IFS) read word by word.
> Additionally, glob expansion will occur.
> 
> I believe the intended use case of these loops is to read lines.

for the patches 1/4 and 2/4 in this series I'm not really sure whether they provide substantial benefit or just solve the problem differently.

Thus, I am going to wait on additional positive feedback here, and if there is none, I consider rejecting them (as that means that there is not benefit to outweigh the risk of regressions).

Best

Adrian

> 
> Signed-off-by: Rosen Penev <rosenp@gmail.com>
> ---
>  package/base-files/files/lib/functions.sh | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/package/base-files/files/lib/functions.sh b/package/base-
> files/files/lib/functions.sh
> index 7da0c872fa..59bb0fe711 100755
> --- a/package/base-files/files/lib/functions.sh
> +++ b/package/base-files/files/lib/functions.sh
> @@ -182,7 +182,8 @@ default_prerm() {
>  	fi
> 
>  	local shell="$(command -v bash)"
> -	for i in $(grep -s "^/etc/init.d/"
> "$root/usr/lib/opkg/info/${pkgname}.list"); do
> +	grep -s "^/etc/init.d/" "$root/usr/lib/opkg/info/${pkgname}.list" |
> while IFS= read -r i
> +	do
>  		if [ -n "$root" ]; then
>  			${shell:-/bin/sh} "$root/etc/rc.common" "$root$i"
> disable
>  		else
> @@ -260,7 +261,8 @@ default_postinst() {
> 
>  		if grep -m1 -q -s "^/etc/uci-defaults/" "$filelist"; then
>  			[ -d /tmp/.uci ] || mkdir -p /tmp/.uci
> -			for i in $(grep -s "^/etc/uci-defaults/" "$filelist"); do
> +			grep -s "^/etc/uci-defaults/" "$filelist" | while IFS=
> read -r i
> +			do
>  				( [ -f "$i" ] && cd "$(dirname $i)" && . "$i" ) &&
> rm -f "$i"
>  			done
>  			uci commit
> @@ -270,7 +272,8 @@ default_postinst() {
>  	fi
> 
>  	local shell="$(command -v bash)"
> -	for i in $(grep -s "^/etc/init.d/" "$root$filelist"); do
> +	grep -s "^/etc/init.d/" "$root$filelist" | while IFS= read -r i
> +	do
>  		if [ -n "$root" ]; then
>  			${shell:-/bin/sh} "$root/etc/rc.common" "$root$i"
> enable
>  		else
> --
> 2.26.2
> 
> 
> _______________________________________________
> openwrt-devel mailing list
> openwrt-devel@lists.openwrt.org
> https://lists.openwrt.org/mailman/listinfo/openwrt-devel
diff mbox series

Patch

diff --git a/package/base-files/files/lib/functions.sh b/package/base-files/files/lib/functions.sh
index 7da0c872fa..59bb0fe711 100755
--- a/package/base-files/files/lib/functions.sh
+++ b/package/base-files/files/lib/functions.sh
@@ -182,7 +182,8 @@  default_prerm() {
 	fi
 
 	local shell="$(command -v bash)"
-	for i in $(grep -s "^/etc/init.d/" "$root/usr/lib/opkg/info/${pkgname}.list"); do
+	grep -s "^/etc/init.d/" "$root/usr/lib/opkg/info/${pkgname}.list" | while IFS= read -r i
+	do
 		if [ -n "$root" ]; then
 			${shell:-/bin/sh} "$root/etc/rc.common" "$root$i" disable
 		else
@@ -260,7 +261,8 @@  default_postinst() {
 
 		if grep -m1 -q -s "^/etc/uci-defaults/" "$filelist"; then
 			[ -d /tmp/.uci ] || mkdir -p /tmp/.uci
-			for i in $(grep -s "^/etc/uci-defaults/" "$filelist"); do
+			grep -s "^/etc/uci-defaults/" "$filelist" | while IFS= read -r i
+			do
 				( [ -f "$i" ] && cd "$(dirname $i)" && . "$i" ) && rm -f "$i"
 			done
 			uci commit
@@ -270,7 +272,8 @@  default_postinst() {
 	fi
 
 	local shell="$(command -v bash)"
-	for i in $(grep -s "^/etc/init.d/" "$root$filelist"); do
+	grep -s "^/etc/init.d/" "$root$filelist" | while IFS= read -r i
+	do
 		if [ -n "$root" ]; then
 			${shell:-/bin/sh} "$root/etc/rc.common" "$root$i" enable
 		else