Shellcheck check suspend resume with audio #1055
Conversation
Fix a variety of quote and quote-related issues per shellcheck's requests. Signed-off-by: Greg Galloway <[email protected]>
The previous source reference for lib.sh triggers shellcheck errors. Fixed a later BASH_SOURCE[0] reference to follow the same pattern. Signed-off-by: Greg Galloway <[email protected]>
There was a problem hiding this comment.
Rewrote conditional of a sof-process-state.sh line (that was basically broken).
It's not obvious why and how it was broken; please explain in the commit message the bug and also update the commit title. When people read "shellcheck fixes" they assume barely any functional change and no bug fix.
Some projects even require an actual bug to be filed before submitting a fix but we're not that pedantic here :-)
| fi | ||
| $(dirname ${BASH_SOURCE[0]})/check-suspend-resume.sh $(echo $opt) || die "suspend resume failed" | ||
| } | ||
| "$(dirname "${BASH_SOURCE[0]}")"/check-suspend-resume.sh "${opt_arr[@]}" || die "suspend resume failed" |
There was a problem hiding this comment.
Please de-duplicate $(dirname "${BASH_SOURCE[0]}) with some new MYDIR variable like other scripts do.
| fi | ||
| if [ ${OPT_VAL['r']} -eq 0 ]; then | ||
| opt="$opt -S ${OPT_VAL['S']} -w ${OPT_VAL['w']}" | ||
| opt_arr+=("-S" "${OPT_VAL['S']}" "-w" "${OPT_VAL['w']}") |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
| else | ||
| opt="$opt -r" | ||
| opt_arr+=("-r") |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
| @@ -78,57 +79,54 @@ fi | |||
|
|
|||
| func_pipeline_export "$tplg" "type:${OPT_VAL['m']} & ${OPT_VAL['P']}" | |||
|
|
|||
| opt_arr=("-l" "${OPT_VAL['l']}") | |||
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
| opt="-l ${OPT_VAL['l']} -T ${OPT_VAL['T']}" | ||
| else | ||
| opt="-l ${OPT_VAL['l']}" | ||
| opt_arr+=("-T" "${OPT_VAL['T']}") |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
greg-intel
force-pushed
the
shellcheck-check-suspend-resume-with-audio
branch
2 times, most recently
from
d7c4eb8
to
4e007b9
Compare
|
32c98bb8a2c56c removes quotes that are added by an earlier commit. Don't add them in the first place. |
There are many online examples that quote each element of arrays. Err'ing on the side of more quotes rather than less didn't seem like a serious problem. http://mywiki.wooledge.org/BashGuide/Arrays for example has this: Where the only elements that would otherwise require quotes are "$USER" and "Big Bad John". Also the first search hit for "Bash arrays" is https://opensource.com/article/18/5/you-dont-know-bash-intro-bash-arrays |
This is a debatable code style question, I agree it can be subjective. In any case:
|
Rewrote 'opt' to be an array. Quoting opt as an argument caused issues that won't manifest when it's an array. Removed unnecessary quoting from array assignments. Signed-off-by: Greg Galloway <[email protected]>
Replaced ps -ef | grep lines with pgrep. Rewrote conditional of a sof-process-state.sh line (that was basically broken). The script had 'set -e' so the command would fail and exit the script. This prevented the follow-up lines, which were checking exit status ($?), from ever being run if the command failed. Removed unused variable assignment pcm). Signed-off-by: Greg Galloway <[email protected]>
There were two references to $(dirname "${BASH_SOURCE[0]}),
so a temp variable was created to hold this reference. The
two references were then re-written.
Signed-off-by: Greg Galloway <[email protected]>
greg-intel
force-pushed
the
shellcheck-check-suspend-resume-with-audio
branch
from
4e007b9
to
1e7f1e2
Compare
|
@marc-hb are there further opens on this PR? I believe I've satisfied all the requests. |
|
SOFCI TEST |
Various shellcheck fixes, including quoting, source references and pgrep.
Work done under ticket #729
Signed-off-by: Greg Galloway [email protected]