Re: allowing multiple PQclear() calls

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Boszormenyi Zoltan <zb(at)cybertec(dot)at>
Cc: Heikki Linnakangas <hlinnakangas(at)vmware(dot)com>, Marko Kreen <markokr(at)gmail(dot)com>, Simon Riggs <simon(at)2ndquadrant(dot)com>, Josh Kupershmidt <schmiddy(at)gmail(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: allowing multiple PQclear() calls
Date: 2013-01-02 16:22:44
Message-ID: 19976.1357143764@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Boszormenyi Zoltan <zb(at)cybertec(dot)at> writes:
> 2013-01-02 16:52 keltezssel, Heikki Linnakangas rta:
>> IMHO this doesn't belong into libpq, the interface is fine as it is. It's the caller's
>> responsibility to set the pointer to NULL after PQclear(), same as it's the caller's
>> responsibility to set a pointer to NULL after calling free(), or to set the fd variable
>> to -1 after calling close(fd). There's plenty of precedence for this pattern, and it
>> shouldn't surprise any programmer.

> Let me quote Simon Riggs here:
>> ... we should introduce a new public API call that is safer,
>> otherwise we get people continually re-inventing new private APIs that
>> Do the Right Thing, as the two other respondents have shown.

Heikki is right and Simon is wrong. This is not a very helpful idea,
and anybody who wants it is probably doing it already anyway.

There might be some value in the proposed macro if using it reliably
stopped bugs of this class, but it won't; the most obvious reason being
that there is seldom only one copy of a given PGconn pointer in an
application. If you just blindly s/PQfinish(x)/PQfinishSafe(&x)/
you will most likely be zapping some low-level function's local
parameter copy, and thus accomplishing nothing of use. You could
possibly make it work fairly consistently if you changed your entire
application to pass around PGconn** instead of PGconn*, but that would
be tedious and somewhat error-prone in itself; and none of the rest of
libpq's API is at all friendly to the idea.

The context where this sort of thing is actually helpful is C++, where
it's possible to introduce enough low-level infrastructure that the
programmer doesn't have to think about what he's doing when using
indirect pointers like that. You can make a C++ wrapper class that is
both guaranteed safe (unlike this) and notationally transparent.
Of course, that has its own costs, but at least the language provides
some support. So it'd be reasonable for libpqxx to do something like
this, but it's not very helpful for libpq to do it. As Heikki says,
there is basically zero precedent for it in libc or other C libraries.
There's a reason for that.

regards, tom lane

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Heikki Linnakangas 2013-01-02 16:23:08 Re: Minor fix in 'clean' action of 'src/backend/Makefile'
Previous Message Heikki Linnakangas 2013-01-02 16:20:21 Re: Minor fix in 'clean' action of 'src/backend/Makefile'