| Message ID | 20231110202137.3978820-5-stefanb@linux.ibm.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Enable shellcheck and fix some issue | expand |
Hi Stefan, On Fri, 2023-11-10 at 15:21 -0500, Stefan Berger wrote: > Address issues raised by shellcheck SC2320: > "This $? refers to echo/printf, not a previous command. > Assign to variable to avoid it being overwritten." > > Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> > --- > tests/Makefile.am | 2 +- > tests/mmap_check.test | 8 +++----- > tests/portable_signatures.test | 9 +++------ > 3 files changed, 7 insertions(+), 12 deletions(-) > > diff --git a/tests/Makefile.am b/tests/Makefile.am > index bcc1ee4..babfa7a 100644 > --- a/tests/Makefile.am > +++ b/tests/Makefile.am > @@ -26,7 +26,7 @@ clean-local: > distclean: distclean-keys > > shellcheck: > - shellcheck -i SC2086,SC2181,SC2046 \ > + shellcheck -i SC2086,SC2181,SC2046,SC2320 \ > functions.sh gen-keys.sh install-fsverity.sh \ > install-mount-idmapped.sh install-openssl3.sh \ > install-swtpm.sh install-tss.sh softhsm_setup \ > diff --git a/tests/mmap_check.test b/tests/mmap_check.test > index 2dd3433..3d2e1b1 100755 > --- a/tests/mmap_check.test > +++ b/tests/mmap_check.test > @@ -97,11 +97,9 @@ check_load_ima_rule() { > > new_policy=$(mktemp -p "$g_mountpoint") > echo "$1" > "$new_policy" > - echo "$new_policy" > /sys/kernel/security/ima/policy > - result=$? > - rm -f "$new_policy" > - > - if [ "$result" -ne 0 ]; then > + if echo "$new_policy" > /sys/kernel/security/ima/policy; then > + rm -f "$new_policy" > + else > echo "${RED}Failed to set IMA policy${NORM}" > return "$HARDFAIL" > fi This isn't equiavlent. $new_policy was previously always removed. > diff --git a/tests/portable_signatures.test b/tests/portable_signatures.test > index 9f3339b..5251211 100755 > --- a/tests/portable_signatures.test > +++ b/tests/portable_signatures.test > @@ -80,7 +80,6 @@ METADATA_CHANGE_FOWNER_2=3002 > > check_load_ima_rule() { > local rule_loaded > - local result > local new_policy > > rule_loaded=$(grep "$1" /sys/kernel/security/ima/policy) > @@ -88,11 +87,9 @@ check_load_ima_rule() { > new_policy=$(mktemp -p "$g_mountpoint") > echo "$1" > "$new_policy" > evmctl sign -o -a sha256 --imasig --key "$key_path" "$new_policy" &> /dev/null > - echo "$new_policy" > /sys/kernel/security/ima/policy > - result=$? > - rm -f "$new_policy" > - > - if [ "$result" -ne 0 ]; then > + if echo "$new_policy" > /sys/kernel/security/ima/policy; then > + rm -f "$new_policy" > + else > echo "${RED}Failed to set IMA policy${NORM}" > return "$FAIL" > fi Same here.
On 11/21/23 18:03, Mimi Zohar wrote: > Hi Stefan, > > On Fri, 2023-11-10 at 15:21 -0500, Stefan Berger wrote: >> Address issues raised by shellcheck SC2320: >> "This $? refers to echo/printf, not a previous command. >> Assign to variable to avoid it being overwritten." >> >> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> >> --- >> tests/Makefile.am | 2 +- >> tests/mmap_check.test | 8 +++----- >> tests/portable_signatures.test | 9 +++------ >> 3 files changed, 7 insertions(+), 12 deletions(-) >> >> diff --git a/tests/Makefile.am b/tests/Makefile.am >> index bcc1ee4..babfa7a 100644 >> --- a/tests/Makefile.am >> +++ b/tests/Makefile.am >> @@ -26,7 +26,7 @@ clean-local: >> distclean: distclean-keys >> >> shellcheck: >> - shellcheck -i SC2086,SC2181,SC2046 \ >> + shellcheck -i SC2086,SC2181,SC2046,SC2320 \ >> functions.sh gen-keys.sh install-fsverity.sh \ >> install-mount-idmapped.sh install-openssl3.sh \ >> install-swtpm.sh install-tss.sh softhsm_setup \ >> diff --git a/tests/mmap_check.test b/tests/mmap_check.test >> index 2dd3433..3d2e1b1 100755 >> --- a/tests/mmap_check.test >> +++ b/tests/mmap_check.test >> @@ -97,11 +97,9 @@ check_load_ima_rule() { >> >> new_policy=$(mktemp -p "$g_mountpoint") >> echo "$1" > "$new_policy" >> - echo "$new_policy" > /sys/kernel/security/ima/policy >> - result=$? >> - rm -f "$new_policy" >> - >> - if [ "$result" -ne 0 ]; then >> + if echo "$new_policy" > /sys/kernel/security/ima/policy; then >> + rm -f "$new_policy" >> + else >> echo "${RED}Failed to set IMA policy${NORM}" >> return "$HARDFAIL" >> fi > > This isn't equiavlent. $new_policy was previously always removed. Uuuh, thanks. Fixed. > >> diff --git a/tests/portable_signatures.test b/tests/portable_signatures.test >> index 9f3339b..5251211 100755 >> --- a/tests/portable_signatures.test >> +++ b/tests/portable_signatures.test >> @@ -80,7 +80,6 @@ METADATA_CHANGE_FOWNER_2=3002 >> >> check_load_ima_rule() { >> local rule_loaded >> - local result >> local new_policy >> >> rule_loaded=$(grep "$1" /sys/kernel/security/ima/policy) >> @@ -88,11 +87,9 @@ check_load_ima_rule() { >> new_policy=$(mktemp -p "$g_mountpoint") >> echo "$1" > "$new_policy" >> evmctl sign -o -a sha256 --imasig --key "$key_path" "$new_policy" &> /dev/null >> - echo "$new_policy" > /sys/kernel/security/ima/policy >> - result=$? >> - rm -f "$new_policy" >> - >> - if [ "$result" -ne 0 ]; then >> + if echo "$new_policy" > /sys/kernel/security/ima/policy; then >> + rm -f "$new_policy" >> + else >> echo "${RED}Failed to set IMA policy${NORM}" >> return "$FAIL" >> fi > > Same here. >
diff --git a/tests/Makefile.am b/tests/Makefile.am index bcc1ee4..babfa7a 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -26,7 +26,7 @@ clean-local: distclean: distclean-keys shellcheck: - shellcheck -i SC2086,SC2181,SC2046 \ + shellcheck -i SC2086,SC2181,SC2046,SC2320 \ functions.sh gen-keys.sh install-fsverity.sh \ install-mount-idmapped.sh install-openssl3.sh \ install-swtpm.sh install-tss.sh softhsm_setup \ diff --git a/tests/mmap_check.test b/tests/mmap_check.test index 2dd3433..3d2e1b1 100755 --- a/tests/mmap_check.test +++ b/tests/mmap_check.test @@ -97,11 +97,9 @@ check_load_ima_rule() { new_policy=$(mktemp -p "$g_mountpoint") echo "$1" > "$new_policy" - echo "$new_policy" > /sys/kernel/security/ima/policy - result=$? - rm -f "$new_policy" - - if [ "$result" -ne 0 ]; then + if echo "$new_policy" > /sys/kernel/security/ima/policy; then + rm -f "$new_policy" + else echo "${RED}Failed to set IMA policy${NORM}" return "$HARDFAIL" fi diff --git a/tests/portable_signatures.test b/tests/portable_signatures.test index 9f3339b..5251211 100755 --- a/tests/portable_signatures.test +++ b/tests/portable_signatures.test @@ -80,7 +80,6 @@ METADATA_CHANGE_FOWNER_2=3002 check_load_ima_rule() { local rule_loaded - local result local new_policy rule_loaded=$(grep "$1" /sys/kernel/security/ima/policy) @@ -88,11 +87,9 @@ check_load_ima_rule() { new_policy=$(mktemp -p "$g_mountpoint") echo "$1" > "$new_policy" evmctl sign -o -a sha256 --imasig --key "$key_path" "$new_policy" &> /dev/null - echo "$new_policy" > /sys/kernel/security/ima/policy - result=$? - rm -f "$new_policy" - - if [ "$result" -ne 0 ]; then + if echo "$new_policy" > /sys/kernel/security/ima/policy; then + rm -f "$new_policy" + else echo "${RED}Failed to set IMA policy${NORM}" return "$FAIL" fi
Address issues raised by shellcheck SC2320: "This $? refers to echo/printf, not a previous command. Assign to variable to avoid it being overwritten." Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> --- tests/Makefile.am | 2 +- tests/mmap_check.test | 8 +++----- tests/portable_signatures.test | 9 +++------ 3 files changed, 7 insertions(+), 12 deletions(-)