[PATCH] Torture script fixes

[Carl-Daniel Hailfinger]

Use mktemp unconditionally for security reasons.
Avoid non-portable seq.

Signed-off-by: Carl-Daniel Hailfinger

Index: flashrom-torturescript_portability/util/flashrom_partial_write_test.sh
===================================================================
--- flashrom-torturescript_portability/util/flashrom_partial_write_test.sh	(Revision 1219)
+++ flashrom-torturescript_portability/util/flashrom_partial_write_test.sh	(Arbeitskopie)
@@ -34,8 +34,12 @@
 echo "testing flashrom binary: ${FLASHROM}"

OLDDIR=$(pwd)
-if [ -z "$TMPDIR" ]; then
-	TMPDIR=$(mktemp -d)	# test data location
+
+# test data location
+TMPDIR=$(mktemp -d -t flashrom_test.XXXXXXXXXX)
+if [ "$?" != "0" ] ; then
+	echo "Could not create temporary directory"
+	exit $EXIT_FAILURE
 fi

ZERO_4K="00_4k.bin"
@@ -65,8 +69,10 @@

# Make 4k worth of 0xff bytes
 echo "begin 640 $FF_4K" > "$FF_4K_TEXT"
-for i in `seq 0 90` ; do
+i=0
+while [ $i -le 90 ] ; do
    echo "M____________________________________________________________" >> "$FF_4K_TEXT"
+	i=$((${i} + 1))
 done
 echo "!_P``" >> "$FF_4K_TEXT"
 echo "`" >> "$FF_4K_TEXT"

[Paul Menzel]

Am Samstag, den 30.10.2010, 02:16 +0200 schrieb Carl-Daniel Hailfinger:
> Use mktemp unconditionally for security reasons.
> Avoid non-portable seq.
>
> Signed-off-by: Carl-Daniel Hailfinger
> Index: flashrom-torturescript_portability/util/flashrom_partial_write_test.sh
>
> --- flashrom-torturescript_portability/util/flashrom_partial_write_test.sh	(Revision 1219)
> +++ flashrom-torturescript_portability/util/flashrom_partial_write_test.sh	(Arbeitskopie)
> @@ -34,8 +34,12 @@
>  echo "testing flashrom binary: ${FLASHROM}"
>
> OLDDIR=$(pwd)
> -if [ -z "$TMPDIR" ]; then
> TMPDIR=$(mktemp -d)	# test data location
>
> +# test data location
> +TMPDIR=$(mktemp -d -t flashrom_test.XXXXXXXXXX)
> +if [ "$?" != "0" ] ; then
> echo "Could not create temporary directory"

Full stop at the end.

Is `mktemp` always installed? If not will there be an error message –
passed down from Bash – indicating that it needs to be installed?
> exit $EXIT_FAILURE
>
> fi
>
> ZERO_4K="00_4k.bin"
> @@ -65,8 +69,10 @@
>
> # Make 4k worth of 0xff bytes
>  echo "begin 640 $FF_4K" > "$FF_4K_TEXT"
> -for i in `seq 0 90` ; do
> +i=0
> +while [ $i -le 90 ] ; do
>    echo "M____________________________________________________________" >> "$FF_4K_TEXT"
> i=$((${i} + 1))
>
> done
>  echo "!_P``" >> "$FF_4K_TEXT"
>  echo "`" >> "$FF_4K_TEXT"

Thanks,

Paul

[David Hendricks]

On Sat, Oct 30, 2010 at 12:43 AM, Paul Menzel <
[e-mail]> wrote:
> Am Samstag, den 30.10.2010, 02:16 +0200 schrieb Carl-Daniel Hailfinger:
> > Use mktemp unconditionally for security reasons.
> > Avoid non-portable seq.
> >
> > Signed-off-by: Carl-Daniel Hailfinger <[e-mail]
> >
> > Index:
>
> flashrom-torturescript_portability/util/flashrom_partial_write_test.sh
> > ===================================================================
>
> flashrom-torturescript_portability/util/flashrom_partial_write_test.sh
>  (Revision 1219)
> > +++
>
> flashrom-torturescript_portability/util/flashrom_partial_write_test.sh
>  (Arbeitskopie)
> > @@ -34,8 +34,12 @@
> >  echo "testing flashrom binary: ${FLASHROM}"
> >
> > OLDDIR=$(pwd)
> > -if [ -z "$TMPDIR" ]; then
> > TMPDIR=$(mktemp -d)     # test data location
> >
> > +# test data location
> > +TMPDIR=$(mktemp -d -t flashrom_test.XXXXXXXXXX)
> > +if [ "$?" != "0" ] ; then
> > echo "Could not create temporary directory"
>
> Full stop at the end.
>
> Is `mktemp` always installed? If not will there be an error message –
> passed down from Bash – indicating that it needs to be installed?

No... it will simply tell the user "Could not create temporary directory."

How about we re-factor the mktemp check as such:
# create location to store temporary data for test usage
TMPDIR=$(mktemp -d -t flashrom_test.XXXXXXXXXX)
if [ "$?" != "0" ] ; then
echo "Could not create temporary directory"
 which mktemp > /dev/null
 if [ "$?" != "0" ] ; then
 echo "mktemp not found"
 fi
 exit $EXIT_FAILURE
fi
> > exit $EXIT_FAILURE
> >
> > fi
> >
> > ZERO_4K="00_4k.bin"
> > @@ -65,8 +69,10 @@
> >
> > # Make 4k worth of 0xff bytes
> >  echo "begin 640 $FF_4K" > "$FF_4K_TEXT"
> > -for i in `seq 0 90` ; do
> > +i=0
> > +while [ $i -le 90 ] ; do
> >       echo
>
> "M____________________________________________________________" >>
> "$FF_4K_TEXT"
> > i=$((${i} + 1))
> >
> > done
> >  echo "!_P``" >> "$FF_4K_TEXT"
> >  echo "`" >> "$FF_4K_TEXT"
>
> Thanks,
>
> Paul
>
> flashrom mailing list
> [e-mail]
> http://www.flashrom.org/mailman/listinfo/flashrom

[Idwer Vollering]

2010/11/1 David Hendricks
> On Sat, Oct 30, 2010 at 12:43 AM, Paul Menzel <
> [e-mail]> wrote:
> > Am Samstag, den 30.10.2010, 02:16 +0200 schrieb Carl-Daniel Hailfinger:
> > > Use mktemp unconditionally for security reasons.
> > > Avoid non-portable seq.
> > >
> > > Signed-off-by: Carl-Daniel Hailfinger <
> >
> > [e-mail]>

Acked-by: Idwer Vollering
> > > Index:
> >
> > flashrom-torturescript_portability/util/flashrom_partial_write_test.sh
> > > ===================================================================
> >
> > flashrom-torturescript_portability/util/flashrom_partial_write_test.sh
> >  (Revision 1219)
> > > +++
> >
> > flashrom-torturescript_portability/util/flashrom_partial_write_test.sh
> >  (Arbeitskopie)
> > > @@ -34,8 +34,12 @@
> > >  echo "testing flashrom binary: ${FLASHROM}"
> > >
> > > OLDDIR=$(pwd)
> > > -if [ -z "$TMPDIR" ]; then
> > > TMPDIR=$(mktemp -d)     # test data location
> > >
> > > +# test data location
> > > +TMPDIR=$(mktemp -d -t flashrom_test.XXXXXXXXXX)
> > > +if [ "$?" != "0" ] ; then
> > > echo "Could not create temporary directory"
> >
> > Full stop at the end.
> >
> > Is `mktemp` always installed? If not will there be an error message –
> > passed down from Bash – indicating that it needs to be installed?

It is safe to assume it is.
> No... it will simply tell the user "Could not create temporary directory."
>
> How about we re-factor the mktemp check as such:
> # create location to store temporary data for test usage
> TMPDIR=$(mktemp -d -t flashrom_test.XXXXXXXXXX)
> if [ "$?" != "0" ] ; then
> echo "Could not create temporary directory"
>  which mktemp > /dev/null
>  if [ "$?" != "0" ] ; then
>  echo "mktemp not found"
>  fi
>  exit $EXIT_FAILURE
> fi
> > > exit $EXIT_FAILURE
> > > fi
> > >
> > > ZERO_4K="00_4k.bin"
> > > @@ -65,8 +69,10 @@
> > >
> > > # Make 4k worth of 0xff bytes
> > >  echo "begin 640 $FF_4K" > "$FF_4K_TEXT"
> > > -for i in `seq 0 90` ; do
> > > +i=0
> > > +while [ $i -le 90 ] ; do
> > >       echo
> >
> > "M____________________________________________________________" >>
> > "$FF_4K_TEXT"
> > > i=$((${i} + 1))
> > >
> > > done
> > >  echo "!_P``" >> "$FF_4K_TEXT"
> > >  echo "`" >> "$FF_4K_TEXT"
> >
> > Thanks,
> >
> > Paul
> >
> > flashrom mailing list
> > [e-mail]
> > http://www.flashrom.org/mailman/listinfo/flashrom
>
> flashrom mailing list
> [e-mail]
> http://www.flashrom.org/mailman/listinfo/flashrom

[David Hendricks]

On Mon, Nov 1, 2010 at 3:30 PM, Idwer Vollering  wrote:
> 2010/11/1 David Hendricks
>
> On Sat, Oct 30, 2010 at 12:43 AM, Paul Menzel <
> > [e-mail]> wrote:
> > > Am Samstag, den 30.10.2010, 02:16 +0200 schrieb Carl-Daniel Hailfinger:
> > > > Use mktemp unconditionally for security reasons.
> > > > Avoid non-portable seq.
> > > >
> > > > Signed-off-by: Carl-Daniel Hailfinger <
> > >
> > > [e-mail]>
>
> Acked-by: Idwer Vollering
> > > > Index:
> > >
> > > flashrom-torturescript_portability/util/flashrom_partial_write_test.sh
> > > > ===================================================================
> > >
> > > flashrom-torturescript_portability/util/flashrom_partial_write_test.sh
> > >  (Revision 1219)
> > > > +++
> > >
> > > flashrom-torturescript_portability/util/flashrom_partial_write_test.sh
> > >  (Arbeitskopie)
> > > > @@ -34,8 +34,12 @@
> > > >  echo "testing flashrom binary: ${FLASHROM}"
> > > >
> > > > OLDDIR=$(pwd)
> > > > -if [ -z "$TMPDIR" ]; then
> > > > TMPDIR=$(mktemp -d)     # test data location
> > > >
> > > > +# test data location
> > > > +TMPDIR=$(mktemp -d -t flashrom_test.XXXXXXXXXX)
> > > > +if [ "$?" != "0" ] ; then
> > > > echo "Could not create temporary directory"
> > >
> > > Full stop at the end.
> > >
> > > Is `mktemp` always installed? If not will there be an error message –
> > > passed down from Bash – indicating that it needs to be installed?
>
> It is safe to assume it is.

To elaborate, "mktemp" is part of the GNU coreutils package, which is pretty
much mandatory on GNU/Linux distros:

Ubuntu example:
dhendrix@amd870:~$ dpkg -S `which mktemp`
coreutils: /bin/mktemp

Gentoo example:
(cros-chroot) dhendrix@thegates ~ $ equery belongs `which mktemp`
 * Searching for /usr/bin/mktemp ...
sys-apps/coreutils-7.5-r1 (/bin/mktemp)
sys-apps/coreutils-7.5-r1 (/usr/bin/mktemp -> /bin/mktemp)

[Carl-Daniel Hailfinger]

On 01.11.2010 23:30, Idwer Vollering wrote:
> 2010/11/1 David Hendricks wrote:
> > On Sat, Oct 30, 2010 at 12:43 AM, Paul Menzel wrote:
> > > Am Samstag, den 30.10.2010, 02:16 +0200 schrieb Carl-Daniel Hailfinger:
> > > > Use mktemp unconditionally for security reasons.
> > > > Avoid non-portable seq.
> > > >
> > > > Signed-off-by: Carl-Daniel Hailfinger
>
> Acked-by: Idwer Vollering

Thanks, committed in r1221.
> > > > --- flashrom-torturescript_portability/util/flashrom_partial_write_test.sh (Revision 1219)
> > > > +++ flashrom-torturescript_portability/util/flashrom_partial_write_test.sh (Arbeitskopie)
> > > > @@ -34,8 +34,12 @@
> > > >  echo "testing flashrom binary: ${FLASHROM}"
> > > >
> > > > OLDDIR=$(pwd)
> > > > -if [ -z "$TMPDIR" ]; then
> > > > TMPDIR=$(mktemp -d)     # test data location
> > > >
> > > > +# test data location
> > > > +TMPDIR=$(mktemp -d -t flashrom_test.XXXXXXXXXX)
> > > > +if [ "$?" != "0" ] ; then
> > > > echo "Could not create temporary directory"
> > >
> > > Full stop at the end.

I just checked the error message from mkdir and it doesn't have a full
stop at the end either. IMHO consistency for the error message style is
a good idea.
> > > Is `mktemp` always installed? If not will there be an error message –
> > > passed down from Bash – indicating that it needs to be installed?
>
> It is safe to assume it is.

Unless you're using some oddball shell on some old QNX version, sh will
detect a missing mktemp and treat it like a failed mktemp.

Regards,
Carl-Daniel