Alex <a...@commandprompt.com> writes:
> Marko Kreen <mark...@gmail.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
*** a/src/interfaces/libpq/fe-connect.c
--- b/src/interfaces/libpq/fe-connect.c
***************
*** 297,302 **** static PQconninfoOption *conninfo_parse(const char *conninfo,
--- 297,304 ----
static PQconninfoOption *conninfo_array_parse(const char *const * keywords,
const char *const * values,
PQExpBuffer errorMessage,
bool use_defaults, int expand_dbname);
+ static bool conninfo_fill_defaults(PQconninfoOption *options,
+ PQExpBuffer errorMessage);
static char *conninfo_getval(PQconninfoOption *connOptions,
const char *keyword);
static void defaultNoticeReceiver(void *arg, const PGresult *res);
***************
*** 836,842 **** PQconndefaults(void)
initPQExpBuffer(&errorBuf);
if (PQExpBufferDataBroken(errorBuf))
return NULL; /* out of memory already :-( */
! connOptions = conninfo_parse("", &errorBuf, true);
termPQExpBuffer(&errorBuf);
return connOptions;
}
--- 838,860 ----
initPQExpBuffer(&errorBuf);
if (PQExpBufferDataBroken(errorBuf))
return NULL; /* out of memory already :-( */
!
! /* Make a working copy of PQconninfoOptions */
! connOptions = malloc(sizeof(PQconninfoOptions));
! if (connOptions == NULL)
! {
! printfPQExpBuffer(&errorBuf,
! libpq_gettext("out of
memory\n"));
! return NULL;
! }
! memcpy(connOptions, PQconninfoOptions, sizeof(PQconninfoOptions));
!
! if (!conninfo_fill_defaults(connOptions, &errorBuf))
! {
! PQconninfoFree(connOptions);
! connOptions = NULL;
! }
!
termPQExpBuffer(&errorBuf);
return connOptions;
}
***************
*** 4002,4008 **** conninfo_parse(const char *conninfo, PQExpBuffer
errorMessage,
char *pname;
char *pval;
char *buf;
- char *tmp;
char *cp;
char *cp2;
PQconninfoOption *options;
--- 4020,4025 ----
***************
*** 4170,4245 **** conninfo_parse(const char *conninfo, PQExpBuffer
errorMessage,
free(buf);
/*
! * Stop here if caller doesn't want defaults filled in.
! */
! if (!use_defaults)
! return options;
!
! /*
! * If there's a service spec, use it to obtain any not-explicitly-given
! * parameters.
*/
! if (parseServiceInfo(options, errorMessage))
{
PQconninfoFree(options);
return NULL;
}
- /*
- * Get the fallback resources for parameters not specified in the
conninfo
- * string nor the service.
- */
- for (option = options; option->keyword != NULL; option++)
- {
- if (option->val != NULL)
- continue; /* Value was in
conninfo or service */
-
- /*
- * Try to get the environment variable fallback
- */
- if (option->envvar != NULL)
- {
- if ((tmp = getenv(option->envvar)) != NULL)
- {
- option->val = strdup(tmp);
- if (!option->val)
- {
- printfPQExpBuffer(errorMessage,
-
libpq_gettext("out of memory\n"));
- PQconninfoFree(options);
- return NULL;
- }
- continue;
- }
- }
-
- /*
- * No environment variable specified or this one isn't set - try
- * compiled in
- */
- if (option->compiled != NULL)
- {
- option->val = strdup(option->compiled);
- if (!option->val)
- {
- printfPQExpBuffer(errorMessage,
-
libpq_gettext("out of memory\n"));
- PQconninfoFree(options);
- return NULL;
- }
- continue;
- }
-
- /*
- * Special handling for user
- */
- if (strcmp(option->keyword, "user") == 0)
- {
- option->val = pg_fe_getauthname(errorMessage);
- continue;
- }
- }
-
return options;
}
--- 4187,4200 ----
free(buf);
/*
! * Fill in defaults if the caller wants that.
*/
! if (use_defaults && !conninfo_fill_defaults(options, errorMessage))
{
PQconninfoFree(options);
return NULL;
}
return options;
}
***************
*** 4262,4268 **** conninfo_array_parse(const char *const * keywords, const
char *const * values,
PQExpBuffer errorMessage, bool
use_defaults,
int expand_dbname)
{
- char *tmp;
PQconninfoOption *options;
PQconninfoOption *str_options = NULL;
PQconninfoOption *option;
--- 4217,4222 ----
***************
*** 4386,4405 **** conninfo_array_parse(const char *const * keywords, const
char *const * values,
PQconninfoFree(str_options);
/*
! * Stop here if caller doesn't want defaults filled in.
*/
! if (!use_defaults)
! return options;
/*
* If there's a service spec, use it to obtain any not-explicitly-given
* parameters.
*/
if (parseServiceInfo(options, errorMessage))
! {
! PQconninfoFree(options);
! return NULL;
! }
/*
* Get the fallback resources for parameters not specified in the
conninfo
--- 4340,4377 ----
PQconninfoFree(str_options);
/*
! * Fill in defaults if the caller wants that.
*/
! if (use_defaults && !conninfo_fill_defaults(options, errorMessage))
! {
! PQconninfoFree(options);
! return NULL;
! }
!
! return options;
! }
!
! /*
! * Fills the connection options array with the default values for unspecified
! * options.
! *
! * Defaults are supplied from a service file, environment variables, etc.
! *
! * Returns TRUE if successful, FALSE otherwise, while filling in errorMessage.
! */
! static bool
! conninfo_fill_defaults(PQconninfoOption *options,
! PQExpBuffer errorMessage)
! {
! PQconninfoOption *option;
! char *tmp;
/*
* If there's a service spec, use it to obtain any not-explicitly-given
* parameters.
*/
if (parseServiceInfo(options, errorMessage))
! return false;
/*
* Get the fallback resources for parameters not specified in the
conninfo
***************
*** 4422,4429 **** conninfo_array_parse(const char *const * keywords, const
char *const * values,
{
printfPQExpBuffer(errorMessage,
libpq_gettext("out of memory\n"));
! PQconninfoFree(options);
! return NULL;
}
continue;
}
--- 4394,4400 ----
{
printfPQExpBuffer(errorMessage,
libpq_gettext("out of memory\n"));
! return false;
}
continue;
}
***************
*** 4440,4447 **** conninfo_array_parse(const char *const * keywords, const
char *const * values,
{
printfPQExpBuffer(errorMessage,
libpq_gettext("out of memory\n"));
! PQconninfoFree(options);
! return NULL;
}
continue;
}
--- 4411,4417 ----
{
printfPQExpBuffer(errorMessage,
libpq_gettext("out of memory\n"));
! return false;
}
continue;
}
***************
*** 4456,4462 **** conninfo_array_parse(const char *const * keywords, const
char *const * values,
}
}
! return options;
}
static char *
--- 4426,4432 ----
}
}
! return true;
}
static char *
*** a/src/interfaces/libpq/fe-connect.c
--- b/src/interfaces/libpq/fe-connect.c
***************
*** 292,297 **** static PGconn *makeEmptyPGconn(void);
--- 292,298 ----
static void fillPGconn(PGconn *conn, PQconninfoOption *connOptions);
static void freePGconn(PGconn *conn);
static void closePGconn(PGconn *conn);
+ static PQconninfoOption *conninfo_init(PQExpBuffer errorMessage);
static PQconninfoOption *conninfo_parse(const char *conninfo,
PQExpBuffer errorMessage, bool use_defaults);
static PQconninfoOption *conninfo_array_parse(const char *const * keywords,
***************
*** 840,853 **** PQconndefaults(void)
return NULL; /* out of memory already :-( */
/* Make a working copy of PQconninfoOptions */
! connOptions = malloc(sizeof(PQconninfoOptions));
if (connOptions == NULL)
- {
- printfPQExpBuffer(&errorBuf,
- libpq_gettext("out of
memory\n"));
return NULL;
- }
- memcpy(connOptions, PQconninfoOptions, sizeof(PQconninfoOptions));
if (!conninfo_fill_defaults(connOptions, &errorBuf))
{
--- 841,849 ----
return NULL; /* out of memory already :-( */
/* Make a working copy of PQconninfoOptions */
! connOptions = conninfo_init(&errorBuf);
if (connOptions == NULL)
return NULL;
if (!conninfo_fill_defaults(connOptions, &errorBuf))
{
***************
*** 4005,4010 **** PQconninfoParse(const char *conninfo, char **errmsg)
--- 4001,4025 ----
}
/*
+ * Makes a copy of PQconninfoOptions array.
+ */
+ static PQconninfoOption *
+ conninfo_init(PQExpBuffer errorMessage)
+ {
+ PQconninfoOption *options;
+
+ options = malloc(sizeof(PQconninfoOptions));
+ if (options == NULL)
+ {
+ printfPQExpBuffer(errorMessage,
+ libpq_gettext("out of
memory\n"));
+ return NULL;
+ }
+ memcpy(options, PQconninfoOptions, sizeof(PQconninfoOptions));
+ return options;
+ }
+
+ /*
* Conninfo parser routine
*
* If successful, a malloc'd PQconninfoOption array is returned.
***************
*** 4026,4039 **** conninfo_parse(const char *conninfo, PQExpBuffer
errorMessage,
PQconninfoOption *option;
/* Make a working copy of PQconninfoOptions */
! options = malloc(sizeof(PQconninfoOptions));
if (options == NULL)
- {
- printfPQExpBuffer(errorMessage,
- libpq_gettext("out of
memory\n"));
return NULL;
- }
- memcpy(options, PQconninfoOptions, sizeof(PQconninfoOptions));
/* Need a modifiable copy of the input string */
if ((buf = strdup(conninfo)) == NULL)
--- 4041,4049 ----
PQconninfoOption *option;
/* Make a working copy of PQconninfoOptions */
! options = conninfo_init(errorMessage);
if (options == NULL)
return NULL;
/* Need a modifiable copy of the input string */
if ((buf = strdup(conninfo)) == NULL)
***************
*** 4252,4266 **** conninfo_array_parse(const char *const * keywords, const
char *const * values,
}
/* Make a working copy of PQconninfoOptions */
! options = malloc(sizeof(PQconninfoOptions));
if (options == NULL)
{
- printfPQExpBuffer(errorMessage,
- libpq_gettext("out of
memory\n"));
PQconninfoFree(str_options);
return NULL;
}
- memcpy(options, PQconninfoOptions, sizeof(PQconninfoOptions));
i = 0;
/* Parse the keywords/values arrays */
--- 4262,4273 ----
}
/* Make a working copy of PQconninfoOptions */
! options = conninfo_init(errorMessage);
if (options == NULL)
{
PQconninfoFree(str_options);
return NULL;
}
i = 0;
/* Parse the keywords/values arrays */
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
