Re: Inconsistent use of test in manual

[Eric Blake]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

According to Reuben Thomas on 3/17/2009 6:23 AM:
> Looking through the manual, I see three types of use of test with =/!=:
> 
> 1. if test $fstype = no
> 
> In this particular case, $fstype is guaranteed to be defined, but isn't
> it better always to write
> 
>    if test "$fstype" = no
> 
> as then one doesn't have to check?

I'm not a fan of excess typing.  So I'd rather make it a point in the
manual that when a variable is entirely under your control, you can take
advantage of that fact.

> 
> 2. if test "$package_foo_enabled" = "yes"
> 
> I like this!

I'm not a fan of the redundant quotes around the yes.  I know that others
are, but there were more places in the manual that avoided the redundant
quotes than used them.  But you are correct that this is not protected
against $package_foo_enabled with odd contents, so I'm adjusting it to add
the x prefix.

> 
> 3. if test "x$with_readline" != xno
> 
> Why not write:
> 
>    if test "$with_readline" != no
> 
> ?

Because if $with_readline starts with -, then this can confuse some
versions of test.  The manual discusses this, under test:
http://www.gnu.org/software/autoconf/manual/autoconf.html#Limitations-of-Builtins

> 
> It would be nice if this could be made consistent.
> 

Here's what I will be pushing soon.

- --
Don't work too hard, make some time for fun as well!

Eric Blake             address@hidden
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Public key at home.comcast.net/~ericblake/eblake.gpg
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iEYEARECAAYFAkm/qdcACgkQ84KuGfSFAYDUzwCgkCDBdCcgcGmy8/wcV+XSe4vw
yqsAn1hyqGyflLAVGlPjoQHoqvQ+wh9j
=SVb6
-----END PGP SIGNATURE-----

>From efb3e8a738994924dfed0e1d73c91e0678622b3b Mon Sep 17 00:00:00 2001
From: Eric Blake <address@hidden>
Date: Tue, 17 Mar 2009 07:39:40 -0600
Subject: [PATCH] Use test consistently in examples.

* doc/autoconf.texi (Subdirectories, Caching Results)
(Common Shell Constructs, Prerequisite Macros, Coding Style)
(Changed Results, Particular Programs, Defining Symbols):
Protect against arbitrary user strings.
(Multiple Cases): Mention why $fstype does not need protection.
Reported by Reuben Thomas.

Signed-off-by: Eric Blake <address@hidden>
---
 ChangeLog         |    8 ++++++++
 doc/autoconf.texi |   34 ++++++++++++++++++----------------
 2 files changed, 26 insertions(+), 16 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 9a5fd5d..05e3a4d 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,13 @@
 2009-03-17  Eric Blake  <address@hidden>

+       Use test consistently in examples.
+       * doc/autoconf.texi (Subdirectories, Caching Results)
+       (Common Shell Constructs, Prerequisite Macros, Coding Style)
+       (Changed Results, Particular Programs, Defining Symbols):
+       Protect against arbitrary user strings.
+       (Multiple Cases): Mention why $fstype does not need protection.
+       Reported by Reuben Thomas.
+
        Improve confusing section names.
        * doc/autoconf.texi (Specifying Names): Rename node...
        (Specifying Target Triplets): ...to this.
diff --git a/doc/autoconf.texi b/doc/autoconf.texi
index e821cbd..a0a19b8 100644
--- a/doc/autoconf.texi
+++ b/doc/autoconf.texi
@@ -3510,7 +3510,7 @@ Subdirectories
 be a literal, i.e., please do not use:

 @example
-if test "$package_foo_enabled" = yes; then
+if test "x$package_foo_enabled" = xyes; then
   $my_subdirs="$my_subdirs foo"
 fi
 AC_CONFIG_SUBDIRS([$my_subdirs])
@@ -3522,7 +3522,7 @@ Subdirectories
 write:

 @example
-if test "$package_foo_enabled" = yes; then
+if test "x$package_foo_enabled" = xyes; then
   AC_CONFIG_SUBDIRS([foo])
 fi
 @end example
@@ -3930,7 +3930,7 @@ Particular Programs

 @example
 AC_PROG_LEX
-if test "$LEX" != flex; then
+if test "x$LEX" != xflex; then
   LEX="$SHELL $missing_dir/missing flex"
   AC_SUBST([LEX_OUTPUT_ROOT], [lex.yy])
   AC_SUBST([LEXLIB], [''])
@@ -8606,7 +8606,9 @@ Multiple Cases
 way to perform the operation has been found yet.

 Here is an example that uses the shell variable @code{fstype} to keep
-track of whether the remaining cases need to be checked.
+track of whether the remaining cases need to be checked.  Note that
+since the value of @code{fstype} is under our control, we don't have to
+use the longer @samp{test "x$fstype" = xno}.

 @example
 @group
@@ -8674,7 +8676,7 @@ Defining Symbols
 @example
 AC_CHECK_FUNC([vprintf], [AC_DEFINE([HAVE_VPRINTF], [1],
                           [Define if vprintf exists.])])
-if test "$ac_cv_func_vprintf" != yes; then
+if test "x$ac_cv_func_vprintf" != xyes; then
   AC_CHECK_FUNC([_doprnt], [AC_DEFINE([HAVE_DOPRNT], [1],
                             [Define if _doprnt exists.])])
 fi
@@ -9016,7 +9018,7 @@ Caching Results
 [AC_CACHE_CHECK([whether true(1) works], [my_cv_shell_true_works],
                 [my_cv_shell_true_works=no
                  (true) 2>/dev/null && my_cv_shell_true_works=yes
-                 if test "$my_cv_shell_true_works" = yes; then
+                 if test "x$my_cv_shell_true_works" = xyes; then
                    AC_DEFINE([TRUE_WORKS], [1],
                              [Define if `true(1)' works properly.])
                  fi])
@@ -9035,7 +9037,7 @@ Caching Results
 [AC_CACHE_CHECK([whether true(1) works], [my_cv_shell_true_works],
                 [my_cv_shell_true_works=no
                  (true) 2>/dev/null && my_cv_shell_true_works=yes])
- if test "$my_cv_shell_true_works" = yes; then
+ if test "x$my_cv_shell_true_works" = xyes; then
    AC_DEFINE([TRUE_WORKS], [1],
              [Define if `true(1)' works properly.])
  fi
@@ -12408,8 +12410,8 @@ Common Shell Constructs
 is empty.  For example,

 @example
-AS_IF([test "$foo" = yes], [HANDLE_FOO([yes])],
-      [test "$foo" != no], [HANDLE_FOO([maybe])],
+AS_IF([test "x$foo" = xyes], [HANDLE_FOO([yes])],
+      [test "x$foo" != xno], [HANDLE_FOO([maybe])],
       [echo foo not specified])
 @end example

@@ -12462,7 +12464,7 @@ Common Shell Constructs
 # This outputs "Have it!".
 header="sys/some file.h"
 AS_TR_SH([HAVE_$header])=yes
-if test "$HAVE_sys_some_file_h" = yes; then echo "Have it!"; fi
+if test "x$HAVE_sys_some_file_h" = xyes; then echo "Have it!"; fi
 @end example
 @end defmac

@@ -13142,7 +13144,7 @@ Prerequisite Macros
 [test "$body_temperature_in_celsius" -gt "38" &&
   dance_floor=occupied])
 AC_DEFUN([NEWTON_JOHN],
-[test "$hair_style" = "curly" &&
+[test "x$hair_style" = xcurly &&
   dance_floor=occupied])
 @end group

@@ -13161,7 +13163,7 @@ Prerequisite Macros
 @example
 AC_INIT([Dance Manager], [1.0], [bug-dance@@example.org])
 RESERVE_DANCE_FLOOR
-if test "$dance_floor" = occupied; then
+if test "x$dance_floor" = xoccupied; then
   AC_MSG_ERROR([cannot pick up here, let's move])
 fi
 @end example
@@ -13174,7 +13176,7 @@ Prerequisite Macros
 @group
 test "$body_temperature_in_Celsius" -gt "38" &&
   dance_floor=occupied
-test "$hair_style" = "curly" &&
+test "x$hair_style" = xcurly &&
   dance_floor=occupied
 fi
 if date | grep '^Sat.*pm' >/dev/null 2>&1; then
@@ -13606,7 +13608,7 @@ Coding Style
 [AC_CACHE_CHECK(for EMX OS/2 environment, ac_cv_emxos2,
 [AC_COMPILE_IFELSE([AC_LANG_PROGRAM(, return __EMX__;)],
 ac_cv_emxos2=yes, ac_cv_emxos2=no)])
-test "$ac_cv_emxos2" = yes && EMXOS2=yes])
+test "x$ac_cv_emxos2" = xyes && EMXOS2=yes])
 @end example

 @noindent
@@ -13621,7 +13623,7 @@ Coding Style
 [AC_COMPILE_IFELSE([AC_LANG_PROGRAM([], [return __EMX__;])],
                    [ac_cv_emxos2=yes],
                    [ac_cv_emxos2=no])])
-test "$ac_cv_emxos2" = yes && EMXOS2=yes[]dnl
+test "x$ac_cv_emxos2" = xyes && EMXOS2=yes[]dnl
 ])# _AC_EMXOS2
 @end example

@@ -21447,7 +21449,7 @@ Changed Results

 @example
 AC_CHECK_FUNCS([syslog])
-if test $ac_cv_func_syslog = no; then
+if test "x$ac_cv_func_syslog" = xno; then
   # syslog is not in the default libraries.  See if it's in some other.
   for lib in bsd socket inet; do
     AC_CHECK_LIB([$lib], [syslog], [AC_DEFINE([HAVE_SYSLOG])
-- 
1.6.1.2