Re: pg_ctl - tighten command parameter checking - Mailing list pgsql-hackers

Oliver, I am going to reject this.  We give them the syntax for the
params.  I don't see a need to check for leading dash to see if they
forgot a param.  I would like to see a more general solution that uses
getopt or something more robust, but moving all that checking to each
param just seems like a waste.

---------------------------------------------------------------------------

Oliver Elphick wrote:
> The attached patch improves the command parameter checking of pg_ctl.
> 
> At present, there is nothing to check that the parameter given with a
> parameter-taking option is actually valid.  For example, -l can be given
> without a following logfile name; on a strict POSIX shell such as ash,
> you will get a subsequent failure because of too many shifts, but bash
> will let it pass without showing any error.  The patch checks that each
> parameter is not empty and is not another option.
> 
> A consequence of this change is that no command-line parameter can begin
> with "-" (except for the parameter to -o); this seems a reasonable
> restriction.
> 
> For consistency and clarity, I have also changed every occurrence of
> "shift ... var=$1" to "var=$2 ... shift".
> 
> -- 
> Oliver Elphick                                Oliver.Elphick@lfix.co.uk
> Isle of Wight                              http://www.lfix.co.uk/oliver
> GPG: 1024D/3E1D0C1C: CA12 09E0 E8D5 8870 5839  932A 614D 4C34 3E1D 0C1C
> 
>      "But as many as received him, to them gave he power to 
>       become the sons of God, even to them that believe on 
>       his name"        John 1:12 

[ text/x-patch is unsupported, treating like TEXT/PLAIN ]

> *** postgresql-7.2.orig/src/bin/pg_ctl/pg_ctl.sh    Sat Sep 29 04:09:32 2001
> --- postgresql-7.2/src/bin/pg_ctl/pg_ctl.sh    Sat Feb 16 10:50:36 2002
> ***************
> *** 127,156 ****
>           exit 0
>           ;;
>       -D)
> -         shift
>           # pass environment into new postmaster
> !         PGDATA="$1"
>           export PGDATA
>           ;;
>       -l)
>           logfile="$2"
>           shift;;
>       -l*)
>           logfile=`echo "$1" | sed 's/^-l//'`
>           ;;
>       -m)
>           shutdown_mode="$2"
>           shift;;
>       -m*)
>           shutdown_mode=`echo "$1" | sed 's/^-m//'`
>           ;;
>       -o)
>           shift
> -         POSTOPTS="$1"
>           ;;
>       -p)
>           shift
> -         po_path="$1"
>           ;;
>       -s)
>           silence_echo=:
> --- 127,197 ----
>           exit 0
>           ;;
>       -D)
>           # pass environment into new postmaster
> !         PGDATA="$2"
> !         if [ -z "$PGDATA" -o `echo x$PGDATA | cut -c1-2` = "x-" ]
> !         then
> !             echo "$CMDNAME: option '-D' specified without a data directory"
> !         exit 1
> !         fi
>           export PGDATA
> +         shift
>           ;;
>       -l)
>           logfile="$2"
> +         if [ -z "$logfile" -o `echo x$logfile | cut -c1-2` = "x-" ]
> +         then
> +             echo "$CMDNAME: option '-l' specified without a logfile"
> +         exit 1
> +         fi
>           shift;;
>       -l*)
>           logfile=`echo "$1" | sed 's/^-l//'`
> +         if [ -z "$logfile" -o `echo x$logfile | cut -c1-2` = "x-" ]
> +         then
> +             echo "$CMDNAME: option '-l' specified without a logfile"
> +         exit 1
> +         fi
>           ;;
>       -m)
>           shutdown_mode="$2"
> +         if [ -z "$shutdown_mode" -o `echo x$shutdown_mode | cut -c1-2` = "x-" ]
> +         then
> +             echo "$CMDNAME: option '-m' specified without a shutdown mode"
> +         exit 1
> +         fi
>           shift;;
>       -m*)
>           shutdown_mode=`echo "$1" | sed 's/^-m//'`
> +         if [ -z "$shutdown_mode" -o `echo x$shutdown_mode | cut -c1-2` = "x-" ]
> +         then
> +             echo "$CMDNAME: option '-m' specified without a shutdown mode"
> +         exit 1
> +         fi
>           ;;
>       -o)
> +         POSTOPTS="$2"
> +         if [ -z "$POSTOPTS" ]
> +         then
> +             echo "$CMDNAME: option '-o' specified without any passed options"
> +         exit 1
> +         fi
> +         if [ `echo x$POSTOPTS | cut -c1-2` != x- ]
> +         then
> +             echo "$CMDNAME: option -o must be followed by one or more further options
> + to pass to the postmaster"
> +         exit 1
> +     fi
>           shift
>           ;;
>       -p)
> +         po_path="$2"
> +         if [ -z "$po_path" -o `echo x$po_path | cut -c1-2` = "x-" ]
> +         then
> +             echo "$CMDNAME: option '-p' specified without a path"
> +         exit 1
> +         fi
>           shift
>           ;;
>       -s)
>           silence_echo=:

> 
> ---------------------------(end of broadcast)---------------------------
> TIP 5: Have you checked our extensive FAQ?
> 
> http://www.postgresql.org/users-lounge/docs/faq.html

--  Bruce Momjian                        |  http://candle.pha.pa.us pgman@candle.pha.pa.us               |  (610)
853-3000+  If your life is a hard drive,     |  830 Blythe Avenue +  Christ can be your backup.        |  Drexel Hill,
Pennsylvania19026