Re: pg_ctl - tighten command parameter checking

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(dot)Elphick(at)lfix(dot)co(dot)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(at)candle(dot)pha(dot)pa(dot)us | (610) 853-3000
+ If your life is a hard drive, | 830 Blythe Avenue
+ Christ can be your backup. | Drexel Hill, Pennsylvania 19026