diff mbox series

[OpenWrt-Devel,6/7] base-files/functions.sh: do not iterate over ls

Message ID 20200117044359.1923979-6-rosenp@gmail.com
State Rejected
Delegated to: Adrian Schmutzler
Headers show
Series [OpenWrt-Devel,1/7] base-files/functions.sh: don't use $var in $(()) | expand

Commit Message

Rosen Penev Jan. 17, 2020, 4:43 a.m. UTC
It does word splitting and glob expansion, which is undesirable.

https://github.com/koalaman/shellcheck/wiki/SC2045

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

Comments

Adrian Schmutzler Jan. 18, 2020, 3:40 p.m. UTC | #1
Hi,

> -----Original Message-----
> From: openwrt-devel [mailto:openwrt-devel-bounces@lists.openwrt.org]
> On Behalf Of Rosen Penev
> Sent: Freitag, 17. Januar 2020 05:44
> To: openwrt-devel@lists.openwrt.org
> Subject: [OpenWrt-Devel] [PATCH 6/7] base-files/functions.sh: do not
> iterate over ls
> 
> It does word splitting and glob expansion, which is undesirable.
> 
> https://github.com/koalaman/shellcheck/wiki/SC2045
> 
> Signed-off-by: Rosen Penev <rosenp@gmail.com>
> ---
>  package/base-files/files/lib/functions.sh | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/package/base-files/files/lib/functions.sh b/package/base-
> files/files/lib/functions.sh
> index 82a2169260..048bfd2b2a 100755
> --- a/package/base-files/files/lib/functions.sh
> +++ b/package/base-files/files/lib/functions.sh
> @@ -283,7 +283,8 @@ default_postinst() {
>  include() {
>  	local file
> 
> -	for file in $(ls $1/*.sh 2>/dev/null); do
> +	for file in "$1"/*.sh; do
> +		[ -e "$file" ] || break

I personally think this command flow is counter-intuitive (having to filter the pattern if no file is found).

I prefer the ls version and would reject that one unless somebody else likes it (and says so until Monday).

Nevertheless, thanks for caring.

Best

Adrian
diff mbox series

Patch

diff --git a/package/base-files/files/lib/functions.sh b/package/base-files/files/lib/functions.sh
index 82a2169260..048bfd2b2a 100755
--- a/package/base-files/files/lib/functions.sh
+++ b/package/base-files/files/lib/functions.sh
@@ -283,7 +283,8 @@  default_postinst() {
 include() {
 	local file
 
-	for file in $(ls $1/*.sh 2>/dev/null); do
+	for file in "$1"/*.sh; do
+		[ -e "$file" ] || break
 		. $file
 	done
 }