Trivial libpq refactoring patch (was: Re: Another review of URI for libpq, v7 submission)

Alex <ash(at)commandprompt(dot)com> writes:

> Marko Kreen <markokr(at)gmail(dot)com> writes:
>
>> On Thu, Mar 15, 2012 at 11:29:31PM +0200, Alex wrote:
>>> https://github.com/a1exsh/postgres/commits/uri
>>
>> The point of the patch is to have one string with all connection options,
>> in standard format, yes? So why does not this work:
>>
>> db = PQconnectdb("postgres://localhost");
>>
>> ?
>
> Good catch.
>
> I've figured out that we'll need a bit more intrusive change than simply
> overriding the expand_dbname check in conninfo_array_parse (like the
> current version does) to support URIs in all PQconnect* variants.
>
> I still need to figure out some details, but this is to give people a
> status update.

While working on this fix I've figured out that I need my
conninfo_uri_parse to support use_defaults parameter, like
conninfo(_array)_parse functions do.

The problem is that the block of code which does the defaults handling
is duplicated in both of the above functions. What I'd like to do is
extract it into a separate function to call. What I wouldn't like is
bloating the original URI patch with this unrelated change.

So here's a trivial patch to do the refactoring. Also, it uses the
newly added conninfo_fill_defaults directly in PQconndefaults, instead
of doing the "parse empty conninfo string" trick. If this could be
applied, I'd rebase my patch against the updated master branch and
submit the new version.

As it goes, the patch adds a little duplication when it comes to
creating a working copy of PQconninfoOptions array. Attached is the
second patch on top of the first one to extract this bit of code into
conninfo_init. Not sure if we should go all the way through this, so
it's not critical if this one is not applied.

--
Regards,
Alex