[ interfaces list added, since we are discussing an incompatible libpq
API change to fix the problem that PQconnectdb() is not thread-safe ]
Magnus Hagander <mha@sollentuna.net> writes:
>> There might be some other problems too, but that's the main one I'm
>> aware of. If we didn't mind breaking existing apps that use
>> PQconndefaults(), it would be straightforward to fix...
> Wouldn't it be possible to do something like this (ok, a little bit ugly,
> but shouldn't break the clients):
> Store the "active" PQconninfoOptions array inside the PGconn struct. That
> way, the user should not require to change anything when doing PQconnectdb()
> and the likes.
I don't think we'd need to store the array on a long-term basis; it is
really only needed as working storage during PQconnectdb. So it could
be malloc'd at PQconnectdb entry and free'd at exit, in the normal case
where it's just supporting a PQconnectdb call. As you saw, that'd be
a pretty straightforward set of changes inside fe-connect.c.
The problem is how should PQconndefaults() act.
> Keep the static array, rename it to PQconninfoDefaultOptions, make it
> contain the *default* options *from the beginning*, and declare it as
> "const". Then have PQconndefaults() return that array.
It's not const because the default values are not constants --- they
depend on environment variables. In theory the results could change
from call to call, if the client program does putenv()'s in between.
I dunno whether there are any thread packages that keep separate
environment-variable sets for each thread, but if so then the results
could theoretically vary across threads.
The most natural thing given the above change would be to say that what
PQconndefaults() returns is a malloc'd array, and the user program is
required to free said array when done with it. (Actually, required to
call some helper routine inside fe-connect.c, which would know to free
the subsidiary variable strings along with the top-level array...)
The breakage here is that existing code won't know to call the free
routine, and will therefore suffer a memory leak of ~ a few hundred bytes
per PQconndefaults() call. It might well be that we could live with
that, since I'll bet that most client programs don't use PQconndefaults
at all, much less call it so many times that a leak of that size would be
a problem. Comments?
What I'm envisioning is a static const array that contains all the
fixed fields of PQconninfoOptions, but the variable fields (just
the "val" current-value field, AFAIR) are permanently NULL. Then
to create a working copy you doptr = malloc(sizeof(PQconninfoOptions));memcpy(ptr, PQconninfoOptions,
sizeof(PQconninfoOptions));
The free routine would run down the work array freeing any non-null val
strings, then free the array itself. No copying or freeing of the
constant subsidiary strings (such as "keyword") is needed.
If you want to work on it, be my guest...
regards, tom lane
