Re: [PATCH 1/4] tests/util/grub-shell-luks-tester: Add missing line to create RET variable in cleanup

[Thomas Schmitt via Grub-devel <grub-devel@gnu.org>]

Hi,

(adding Vladimir Serbinenko to Cc)

Glenn Washburn wrote:
> --- a/tests/util/grub-shell-luks-tester.in
> +++ b/tests/util/grub-shell-luks-tester.in
> @@ -143,6 +143,7 @@ fi
>
>  # Make sure that the dm-crypto device is shutdown
>  cleanup() {
> +    RET=$?
>      if [ -e "$luksdev" ]; then
>          cryptsetup close "$luksdev"
>      fi

The change itself looks good to me.


But i think that there is
  set -e
missing in
  tests/util/grub-shell-luks-tester.in
and possibly in
  tests/grub_cmd_cryptomount.in

At least inserted commands:
  echo "grub_cmd_cryptomount.in: $(set -o | grep errexit)"
  echo "grub-shell-luks-tester.in: $(set -o | grep errexit)" >>/tmp/grub-shell-luks-tester.errexit.log
say in grub_cmd_cryptomount.log and grub-shell-luks-tester.errexit.log:
  grub_cmd_cryptomount: errexit           off
  grub-shell-luks-tester.in: errexit              off

The neighbors of grub-shell-luks-tester.in have such a "set -e":
  tests/util/grub-fs-tester.in
  tests/util/grub-shell-tester.in
  tests/util/grub-shell.in

Many tests/*.in have it too. So i propose to add it to
  tests/grub_cmd_cryptomount.in
as well.


I came to this while trying whether it is it guaranteed that "$?" is
always meaningful, given the fact that cleanup() can be called from
various places in the script by
  trap cleanup EXIT INT TERM KILL QUIT

My experiment was to press Ctrl+C while this shell code was sleeping

  cleanup() {
    RET=$?
    echo "RET=$RET"
  }
  trap cleanup EXIT INT TERM KILL QUIT
  sleep 10
  ls /notexisting

I got two output lines:

  RET=130
  RET=2

So without "set -e" cleanup() can be called more than once.
I think that grub-shell-luks-tester.in does not expects this.


Have a nice day :)

Thomas


_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel