| Message ID | 6a802aa37758e5a7e6aa5de294634f5518005e2b.1660064925.git.dxu@dxuuu.xyz (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | BPF |
| Headers | show |
| Series | bpf/selftests: Small vmtest.sh fixes | expand |
On Tue, Aug 09, 2022 at 11:11:09AM -0600, Daniel Xu wrote: > Set the exit trap only after argument parsing is done. This way argument > parse failure or `-h` will not require sudo. > > Reasoning is that it's confusing that a help message would require root > access. > > Signed-off-by: Daniel Xu <dxu@dxuuu.xyz> > --- > tools/testing/selftests/bpf/vmtest.sh | 32 +++++++++++++-------------- > 1 file changed, 16 insertions(+), 16 deletions(-) > > diff --git a/tools/testing/selftests/bpf/vmtest.sh b/tools/testing/selftests/bpf/vmtest.sh > index b86ae4a2e5c5..976ef7585b33 100755 > --- a/tools/testing/selftests/bpf/vmtest.sh > +++ b/tools/testing/selftests/bpf/vmtest.sh > @@ -307,6 +307,20 @@ update_kconfig() > fi > } > > +catch() > +{ > + local exit_code=$1 > + local exit_status_file="${OUTPUT_DIR}/${EXIT_STATUS_FILE}" > + # This is just a cleanup and the directory may > + # have already been unmounted. So, don't let this > + # clobber the error code we intend to return. > + unmount_image || true > + if [[ -f "${exit_status_file}" ]]; then > + exit_code="$(cat ${exit_status_file})" > + fi > + exit ${exit_code} > +} > + > main() > { > local script_dir="$(cd -P -- "$(dirname -- "${BASH_SOURCE[0]}")" && pwd -P)" > @@ -353,6 +367,8 @@ main() > done > shift $((OPTIND -1)) > > + trap 'catch "$?"' EXIT > + > if [[ $# -eq 0 && "${debug_shell}" == "no" ]]; then > echo "No command specified, will run ${DEFAULT_COMMAND} in the vm" > else > @@ -409,20 +425,4 @@ main() > fi > } > > -catch() > -{ > - local exit_code=$1 > - local exit_status_file="${OUTPUT_DIR}/${EXIT_STATUS_FILE}" > - # This is just a cleanup and the directory may > - # have already been unmounted. So, don't let this > - # clobber the error code we intend to return. > - unmount_image || true > - if [[ -f "${exit_status_file}" ]]; then > - exit_code="$(cat ${exit_status_file})" > - fi > - exit ${exit_code} > -} > - > -trap 'catch "$?"' EXIT > - > main "$@" > -- > 2.37.1 > Makes sense and looks good to me. Acked-by: Daniel Müller <deso@posteo.net>
diff --git a/tools/testing/selftests/bpf/vmtest.sh b/tools/testing/selftests/bpf/vmtest.sh index b86ae4a2e5c5..976ef7585b33 100755 --- a/tools/testing/selftests/bpf/vmtest.sh +++ b/tools/testing/selftests/bpf/vmtest.sh @@ -307,6 +307,20 @@ update_kconfig() fi } +catch() +{ + local exit_code=$1 + local exit_status_file="${OUTPUT_DIR}/${EXIT_STATUS_FILE}" + # This is just a cleanup and the directory may + # have already been unmounted. So, don't let this + # clobber the error code we intend to return. + unmount_image || true + if [[ -f "${exit_status_file}" ]]; then + exit_code="$(cat ${exit_status_file})" + fi + exit ${exit_code} +} + main() { local script_dir="$(cd -P -- "$(dirname -- "${BASH_SOURCE[0]}")" && pwd -P)" @@ -353,6 +367,8 @@ main() done shift $((OPTIND -1)) + trap 'catch "$?"' EXIT + if [[ $# -eq 0 && "${debug_shell}" == "no" ]]; then echo "No command specified, will run ${DEFAULT_COMMAND} in the vm" else @@ -409,20 +425,4 @@ main() fi } -catch() -{ - local exit_code=$1 - local exit_status_file="${OUTPUT_DIR}/${EXIT_STATUS_FILE}" - # This is just a cleanup and the directory may - # have already been unmounted. So, don't let this - # clobber the error code we intend to return. - unmount_image || true - if [[ -f "${exit_status_file}" ]]; then - exit_code="$(cat ${exit_status_file})" - fi - exit ${exit_code} -} - -trap 'catch "$?"' EXIT - main "$@"
Set the exit trap only after argument parsing is done. This way argument parse failure or `-h` will not require sudo. Reasoning is that it's confusing that a help message would require root access. Signed-off-by: Daniel Xu <dxu@dxuuu.xyz> --- tools/testing/selftests/bpf/vmtest.sh | 32 +++++++++++++-------------- 1 file changed, 16 insertions(+), 16 deletions(-)