[m-rev.] for review: scripts style

[Ralph Becket]

Zoltan Somogyi, Monday, 13 November 2006:
> For review by anyone. Shell scripts are much easier to screw up without
> detection than Mercury or even C programs, so I would appreciate a second
> eye going over them.

I can't see any screw-ups in there, but there are several places where
environment variables are not quoted in `test's and `case's, which might
be worth fixing.

> Index: scripts/mercury_update_interface.in
> ===================================================================
> RCS file: /home/mercury/mercury1/repository/mercury/scripts/mercury_update_interface.in,v
> retrieving revision 1.6
> diff -u -b -r1.6 mercury_update_interface.in
> --- scripts/mercury_update_interface.in	27 Jul 1997 15:09:32 -0000	1.6
> +++ scripts/mercury_update_interface.in	12 Nov 2006 08:34:05 -0000
> @@ -1,4 +1,5 @@
>  #! /bin/sh
> +# vim: ts=4 sw=4 et
>  # @configure_input@
>  #---------------------------------------------------------------------------#
>  # Copyright (C) 1995-1997 The University of Melbourne.
> @@ -22,19 +23,21 @@
>  
>  verbose=false
>  
> -if [ $# -ge 1 ] && [ "$1" = "-v" ]; then
> +if test $# -ge 1 -a "$1" = "-v"
> +then
>  	verbose=true
>  	shift
>  fi

Although it should always be okay to use $# unquoted in tests, it's
probably a better convention to quote *all* shell variables in these
cases.

> -if [ $# -ne 1 ]; then
> +if test $# -ne 1
> +then
>  	echo "Usage: `basename $0` filename" 1>&2
>  	exit 1
>  fi
>  
>  filename="$1"
>  	
> -if	[ ! -f "$filename" ]
> +if test ! -f "$filename"
>  then
>  	$verbose && echo "creating \`$filename'." 1>&2
>  	mv -f "$filename.tmp" "$filename"
> Index: scripts/mgnuc.in
> ===================================================================
> RCS file: /home/mercury/mercury1/repository/mercury/scripts/mgnuc.in,v
> retrieving revision 1.118
> diff -u -b -r1.118 mgnuc.in
> --- scripts/mgnuc.in	1 Nov 2006 02:31:18 -0000	1.118
> +++ scripts/mgnuc.in	12 Nov 2006 08:44:13 -0000
> @@ -138,7 +138,8 @@
>      compiler search paths.
>  "
>  
> -while : ; do
> +while :
> +do
>      case "$1" in
>          -h|--help|"-?")
>              echo "$Help"
> @@ -235,12 +236,10 @@
>      false)      LLDEBUG_OPTS="" ;;
>  esac
>  
> -#
> -# convert grade mgnuc options into gcc options
> +# Convert grade mgnuc options into gcc options.
>  #
>  # IMPORTANT: any changes here may require similar changes to all the files
>  # mentioned in runtime/mercury_grade.h.
> -#
>  
>  case $highlevel_code in
>      true)       HLC_OPTS="-DMR_HIGHLEVEL_CODE" ;;
> @@ -274,15 +273,18 @@
>  
>  case $thread_safe in
>      true)       THREAD_OPTS="$CFLAGS_FOR_THREADS"
> -                case $FULLARCH in *linux*)
> +            case $FULLARCH in
> +                *linux*)

Should $FULLARCH be quoted here (and similarly for other variables in
other cases)?

>                      # Don't use -ansi under Linux or we get parse errors
>                      # at sigset_t in the pthreads headers. This doesn't seem
>                      # to be necessary for recent versions of Linux/glibc
>                      # (e.g. glibc 2.1.2), but I've left it in so we can remain
>                      # compatible with older versions.
>                      ANSI_OPTS=""
> -                esac ;;
> -    false)      THREAD_OPTS="" ;;
> +            esac
> +            ;;

Shouldn't there be a ;; before that esac, closing the switch case?

[There are several more cases of unquoted variables in tests etc., but
it's more useful to search for them than for me to list them here.]

-- Ralph
--------------------------------------------------------------------------
mercury-reviews mailing list
Post messages to:       mercury-reviews at csse.unimelb.edu.au
Administrative Queries: owner-mercury-reviews at csse.unimelb.edu.au
Subscriptions:          mercury-reviews-request at csse.unimelb.edu.au
--------------------------------------------------------------------------