Last minute mini-proposal (I know, I know) for PQexecf()

From: <korryd(at)enterprisedb(dot)com>
To: <pgsql-hackers(at)postgresql(dot)org>
Subject: Last minute mini-proposal (I know, I know) for PQexecf()
Date: 2007-03-31 00:07:53
Message-ID: 1175299673.6784.26.camel@sakai.localdomain
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

While cleaning up some pg_migrator code
( it occurred to me that a
typical libpq client application spends a lot of code constructing SQL
commands. The code typically looks like this:

a) allocate enough room to hold the command
b) sprintf( command, text, argument, argument, argument, ... )
c) PQexec( conn, command )
d) free( command )

In most cases, the amount of memory that you allocate in step a) is just
an educated guess. It's typically more room than you need,
occassionally less room than you need (and you get a buffer overflow
exploit), and it's rarely maintained properly when you modify the
command text (or the argument list).

I'd like to see a new variant on PQexec():

PGresult * PQexecf(PGconn *conn, const char *fmt, ...);

PQexecf() simply performs steps a, b, c, and d for you. And you call it
like this:

PQexecf( conn, text, argument, argument, argument, ... )

PQexecf() is just a wrapper around the already existing
createPQExpBuffer(), enlargePQExpBuffer(), printfPQExpBuffer(), and
PQexec() so it introduces no new code (other than assembling the
wrapper) and doesn't change any existing code. PQexecf() is similar to
PQexecParams() but it much simpler to use (and should be very familiar
to C programmers). PQexecf() is not intended as a replacement for
PQprepare() and PQexecPrepared() - you should use prepare/exec when you
want to execute a command many times.

I could eliminate a lot of client-side code if PQexecf() were available
- and the code that I could remove is the code that's most likely to be
buggy and least likely to be properly maintained.

I've thrown together an UNTESTED prototype (below), just to get the idea
across - you'll recognize that most of this code is identical to
printPQExpBuffer(). In the prototype, I'm keeping a static PQExpBuffer
that grows to the hold the largest string ever required by the client
application - that part seems to be a point for discussion, but since
the detail is hidden in the implementation, we could adjust the code
later if necessary (without changing the interface).

Of course, I could include an implementation of PQexecf() in each of my
client applications if it were not available in libpq, but that would be
silly and I'd have to invent my own createPQExpBuffer() /
enlargePQExpBuffer() code since those functions are not an official part
of libpq (and won't even be available to a Win32 client application).

Is it just too late to even think about this for 8.3? (Bruce laughed at
me when I suggested the idea :-)

-- Korry

PGresult *
PQexecf(PGconn *conn, const char *fmt, ...)
static PQExpBuffer str;
va_list args;

if (str == NULL)
str = createPQExpBuffer();

for (;;)
* Try to format the given string into the available space; but if
* there's hardly any space, don't bother trying, just fall through to
* enlarge the buffer first.
if (str->maxlen > str->len + 16)
size_t avail = str->maxlen - str->len - 1;
int nprinted;

va_start(args, fmt);
nprinted = vsnprintf(str->data + str->len, avail, fmt, args);

* Note: some versions of vsnprintf return the number of chars
* actually stored, but at least one returns -1 on failure. Be
* conservative about believing whether the print worked.
if (nprinted >= 0 && nprinted < (int) avail - 1)
/* Success. Note nprinted does not include trailing null. */
str->len += nprinted;
/* Double the buffer size and try again. */
if (!enlargePQExpBuffer(str, str->maxlen))
return PQmakeEmptyPGresult(conn, PGRES_FATAL_ERROR); /* oops, out of
memory */

return PQexec(conn, str->data);


Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2007-03-31 00:30:22 Re: Last minute mini-proposal (I know, I know) for PQexecf()
Previous Message Florian G. Pflug 2007-03-30 22:51:36 Re: Minor changes to Recovery related code