make check as superuser leaves empty directories in /-directory

make check as superuser leaves empty directories in /-directory

[Thomas Schmitt via Grub-devel]

Hi,

i find in the root directory of my system a lot of empty directories
like
  /1678114331.LUKS1_test_with_twofish_cipher
  /1678114333.LUKS1_test_key_file_support

I believe they come from
  tests/grub_cmd_cryptomount.in
where i read

  eval testcase "'LUKS1 test with twofish cipher:'" \
    @builddir@/grub-shell-luks-tester --luks=1 $COMMON_OPTS \
        "--cs-opts='--cipher twofish-xts-plain64'"

  eval testcase "'LUKS1 test key file support:'" \
    @builddir@/grub-shell-luks-tester --luks=1 $COMMON_OPTS \
        --keyfile

In the function _testcase() of that file i see

  mkdir -p "$TMPDIR"

but the whole file contains no rmdir command.
The path "$TMPDIR" is composed by
  # Create a subdir in TMPDIR for each testcase
  _TMPDIR=$TMPDIR
  TMPDIR=$TMPDIR/`echo -n "$(date +%s).$LOGPREFIX" | sed -e 's,[ /],_,g' -e 's,:$,,g'`


So "$TMPDIR" would initially have been empty. I wonder why.

I fail to see what else but mkdir is done to "$TMPDIR". So i cannot tell
when it can be removed.


"git blame" points to commit a7b540e6 by Glenn Washburn. (Cc'ed)
The later commit 56b367d by Glenn Washburn is old enough to have been in
effect when i first ran "make check" as superuser.

Commit 5a311d0 by Gary Lin is too young for having caused my oldest
/LUKS* directories.

----------------------------------------------------------------------
Why i think GRUB "make check" as superuser is to blame:

The timestamps of the directories correlate with my memory of when i ran
"make check" as superuser to check the impact of my proposed patch:

  https://lists.gnu.org/archive/html/grub-devel/2024-06/msg00197.html
  "[PATCH 0/2] grub-fstest: Show error message if command causes
   grub_errno"
  (The first paragraph of the cover letter is in
    https://lists.gnu.org/archive/html/grub-devel/2024-06/msg00201.html
  )

I ran "make check" as superuser last year with Debian 11 and this year
with Debian 12. Both occasions obviously left their /*.LUKS* directories.

A google search with "LUKS1_test_detached_header_support" leads
to an earlier patch proposal by Glenn Washburn of 2020:
  https://www.mail-archive.com/grub-devel@gnu.org/msg30613.html


Have a nice day :)

Thomas


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

Re: make check as superuser leaves empty directories in /-directory

[Thomas Schmitt via Grub-devel]

Hi,

i believe to have found out what's wrong with TMPDIR in
  tests/grub_cmd_cryptomount.in

It does not get assigned a default value and is used without such a
value. Many other tests have the following gesture before using $TMPDIR:

  : "${TMPDIR=/tmp}"

But grub_cmd_cryptomount.in has not and uses ${TMPDIR:-/tmp} only at a
single occasion when running mktemp. The mkdir step is without such a
safety net:

  mkdir -p "$TMPDIR"


I fail to find this special expansion ${X=Y} in man bash, but experiments
show that it works like {X:=Y} with the difference that a defined empty X
is not filled with "Y".

(I do not think that this respecting of empty set variables is desirable
in GRUB tests. Empty TMPDIR is bad regardless of being set or unset.)


Have a nice day :)

Thomas


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

Re: make check as superuser leaves empty directories in /-directory

[Thomas Schmitt via Grub-devel]

Hi,

with the freshly submitted patch applied, i see as superuser:

  # make check TESTS=grub_cmd_cryptomount
  ...
  PASS: grub_cmd_cryptomount
  ...
  # echo $?
  0
  #

No new directories appeared in the root directory. During the "make check"
run i could spot single files /tmp/*LUKS* which did not exist any more
soon after.

By help of Greg Wooledge on debian-user mailing i was able to find the
description of ${TMPDIR=/tmp} in man bash and man dash.
"=" sets the variable to /tmp only if TMPDIR is not set.
":=" sets also if it is set to empty text.

The use of TMPDIR in tests/ is mainly made safe by ${TMPDIR:-/tmp}, not
by ${TMPDIR-/tmp}. So i deem ${TMPDIR:=/tmp} the right thing to do,
if not every expansion of $TMPDIR shall be made safe individually.

If the gnulib tests are run by "make check", then one should review
the use of ${TMPDIR=/tmp} in
  ./gnulib/tests/test-copy-acl.sh
  ./gnulib/tests/test-file-has-acl.sh
  ./gnulib/tests/test-set-mode-acl.sh
  ./gnulib/tests/test-copy-file.sh
  ./gnulib/tests/test-parse-duration.sh


Have a nice day :)

Thomas


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