Re: [LTP] [PATCH] Remove files which does not have a dmesg entry

[Garrett C. <yan...@gm...>]

On Mon, Jun 28, 2010 at 4:52 AM, Subrata Modak
<su...@li...> wrote:
> Hi,
>
> The recently introduced option:
>
> -K DMESG_LOG_DIR
>        Log Kernel messages generated for each test cases inside this directory,
>
> when used will generate huge no. of files inside the DMESG_DIR. Many of
> them will be ZERO size as we do not expect KERNEL MESSAGE to be generated
> against all the LTP TESTS that we will run. It is better to prune them, and
> let only those files exist which will have real entries. The following
> patch fixes the same.
>
> Signed-off-by: Subrata Modak <su...@li...>,
> ---
>
> --- ltp/runltp.orig     2010-06-28 10:02:42.000000000 +0530
> +++ ltp/runltp  2010-06-28 10:21:04.000000000 +0530
> @@ -858,6 +858,30 @@ main()
>     cd ..
>     echo "LTP Version: $version_date"
>
> +       # DMESG_DIR will contain hell lot of files which will not have any entry
> +       # since kernel message(s) will not be generated for every test. The zero size
> +       # files will clog the directory. Lets keep only those fules which have real
> +       # kernel message entries. If DMESG_DIR directroy is empty after test run, it
> +       # will indicate that no kernel message was generated during test run
> +

This comment could be abbreviated to:

# $DMESG_DIR is used to cache messages obtained from dmesg after a test run.
# Proactively reap all of the 0-byte files in $DMESG_DIR as they have zero value
# and only clutter up the filesystem.

> +       if [ $ALT_DMESG_OUT -eq 1 ] ; then
> +               PWD=`pwd`

Please use a different variable name. This obfuscates the meaning of
the shell set value.

> +               cd $DMESG_DIR

What if this fails? Won't this nuke all of the files in the current
directory? Are you checking to ensure that this isn't empty beforehand
as well? I hate it when scripts nuke files like this (I lost several
hours of work last week because of a similar mistake).

> +               for dmesg_file in `ls`
> +               do
> +                       dmesg_file_size=`wc -c $dmesg_file | cut -d ' ' -f1 | tr -d '\n'`
> +                       if [ $dmesg_file_size -eq 0 ]; then
> +                               rm -rf $dmesg_file
> +                       fi
> +               done
> +               cd $PWD
> +       fi

Thanks,
-Garrett