Re: Sanity checking for ./configure options?

On 2/22/16 6:24 PM, Jim Nasby wrote:
> On 2/5/16 10:08 AM, David Fetter wrote:
>> On Wed, Feb 03, 2016 at 06:02:57PM -0600, Jim Nasby wrote:
>>> I just discovered that ./configure will happily accept
>>> '--with-pgport=' (I
>>> was actually doing =$PGPORT, and didn't realize $PGPORT was empty).
>>> What you
>>> end up with is a compile error in guc.c, with no idea why it's
>>> broken. Any
>>> reason not to have configure or at least make puke if pgport isn't
>>> valid?
>>
>> That seems like a good idea.
>
> Patch attached. I've verified it with --with-pgport=, =0, =77777 and =1.
> It catches what you'd expect it to.

Your code and comments suggest that you can specify the port to
configure by setting PGPORT, but that is not the case.

test == is not portable (bashism).

Error messages should have consistent capitalization.

Indentation in configure is two spaces.

> As the comment states, it doesn't catch things like --with-pgport=1a in
> configure, but the compile error you get with that isn't too hard to
> figure out, so I think it's OK.

Passing a non-integer as argument will produce an error message like
(depending on shell)

./configure: line 3107: test: 11a: integer expression expected

but will not actually abort configure.

It would work more robustly if you did something like this

elif test "$default_port" -ge "1" -a "$default_port" -le "65535"; then
:
else
AC_MSG_ERROR([port must be between 1 and 65535])
fi

but that still leaks the shell's error message.

There is also the risk of someone specifying a number with a leading
zero, which C would interpret as octal but the shell would not.

To make this really robust, you might need to do pattern matching on the
value.