Thread: proposed PQconnectdbParams macros (was Re: [BUGS] BUG #5304: psql using conninfo fails in connecting to the server)

[moving to hackers]

On 02/02/2010 10:23 PM, Tom Lane wrote:
> Joe Conway <mail@joeconway.com> writes:
>> Should I also be looking to replace all (or most) other instances of
>> PQsetdbLogin()?
>
> I think we at least wanted to fix pg_dump(all)/pg_restore.  Not sure if
> the others are worth troubling over.

Any objection if I add these two macros to libpq-fe.h?
-------------------
+ /* Useful macros for PQconnectdbParams() and PQconnectStartParams() */
+ #define DECL_PARAMS_ARRAYS(PARAMS_ARRAY_SIZE) \
+       const char **keywords = pg_malloc(PARAMS_ARRAY_SIZE * \
+                                         sizeof(*keywords)); \
+       const char **values = pg_malloc(PARAMS_ARRAY_SIZE * \
+                                       sizeof(*values))
+ #define SET_PARAMS_ARRAYS(__kv_idx__,__kv_kw__,__kv_v__) \
+       keywords[__kv_idx__]    = __kv_kw__; \
+       values[__kv_idx__]      = __kv_v__


That way this:
-------------------
! #define PARAMS_ARRAY_SIZE    7
!     const char **keywords = pg_malloc(PARAMS_ARRAY_SIZE *
!                                       sizeof(*keywords));
!     const char **values = pg_malloc(PARAMS_ARRAY_SIZE *
!                                     sizeof(*values));
!
!     keywords[0]    = "host";
!     values[0]        = options.host;
!     keywords[1]    = "port";
!     values[1]        = options.port;
!     keywords[2]    = "user";
!     values[2]        = options.username;
!     keywords[3]    = "password";
!     values[3]        = password;
!     keywords[4]    = "dbname";
!     values[4]        = (options.action == ACT_LIST_DB &&
!                          options.dbname == NULL) ?
!                          "postgres" : options.dbname;
!     keywords[5]    = "fallback_application_name";
!     values[5]        = pset.progname;
!     keywords[6]    = NULL;
!     values[6]        = NULL;


becomes this:
-------------------
!     DECL_PARAMS_ARRAYS(7);

!     SET_PARAMS_ARRAYS(0,"host",options.host);
!     SET_PARAMS_ARRAYS(1,"port",options.port);
!     SET_PARAMS_ARRAYS(2,"user",options.username);
!     SET_PARAMS_ARRAYS(3,"password",password);
!     SET_PARAMS_ARRAYS(4,"dbname",(options.action == ACT_LIST_DB &&
!                                   options.dbname == NULL) ?
!                                   "postgres" : options.dbname);
!     SET_PARAMS_ARRAYS(5,"fallback_application_name",pset.progname);
!     SET_PARAMS_ARRAYS(6,NULL,NULL);


I suppose if we do this maybe we should also do:
#define PARAMS_KEYWORDS keywords
#define PARAMS_VALUES values

so the subsequent use looks like:

pset.db = PQconnectdbParams(PARAMS_KEYWORDS, PARAMS_VALUES, true);

To me it seems easier to read and less error prone. Comments?

Joe

Joe Conway <mail@joeconway.com> writes:
> Any objection if I add these two macros to libpq-fe.h?
> -------------------
> + /* Useful macros for PQconnectdbParams() and PQconnectStartParams() */
> + #define DECL_PARAMS_ARRAYS(PARAMS_ARRAY_SIZE) \
> +       const char **keywords = pg_malloc(PARAMS_ARRAY_SIZE * \
> +                                         sizeof(*keywords)); \
> +       const char **values = pg_malloc(PARAMS_ARRAY_SIZE * \
> +                                       sizeof(*values))
> + #define SET_PARAMS_ARRAYS(__kv_idx__,__kv_kw__,__kv_v__) \
> +       keywords[__kv_idx__]    = __kv_kw__; \
> +       values[__kv_idx__]      = __kv_v__

Yes.  Exposing pg_malloc is 100% wrong, and would be even if you'd
remembered the subsequent free() ;-)

It does not seem especially useful either.  In most cases the array size
is constant and there's no reason to not just make it a local in the
calling function.
        regards, tom lane