Re: Proposal for better PQExpBuffer out-of-memory behavior

From: Sam Mason <sam(at)samason(dot)me(dot)uk>
To: pgsql-hackers(at)postgresql(dot)org
Subject: Re: Proposal for better PQExpBuffer out-of-memory behavior
Date: 2008-11-26 12:12:31
Message-ID: 20081126121231.GU2459@frubble.xen.chris-lamb.co.uk
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tue, Nov 25, 2008 at 10:33:05AM -0500, Tom Lane wrote:
> I've been chewing on the problem described here:
> http://archives.postgresql.org/pgsql-general/2008-11/msg01220.php
>
> It's not particularly easy to fix without making annoyingly large
> changes to the API for PQExpBuffer.

Yup, I've just realized that my very naive suggestion have having a
matching function to return something useful wouldn't be good as almost
all the functions return void and would introduce the most enormous
duplicity.

> The best idea I have come up with so far goes like this:
>
> * Upon failure to malloc or realloc the buffer for a PQExpBuffer,
> the pqexpbuffer.c code should release whatever buffer it might have
> had and set
> data = pointer to empty, statically allocated string
> len = 0
> maxlen = 0
> This is distinguishable from the normal non-error case because maxlen
> can never be zero in non-error cases.
>
> * All subsequent operations except resetPQExpBuffer will do nothing
> to such a PQExpBuffer. resetPQExpBuffer will attempt to restore the
> string to "normal" empty status by allocating a new default-size buffer.

Sounds like a reasonable compromise. Would it be better to have this
string be something like "## out of memory in enlargePQExpBuffer ##"?
That way, if something doesn't check correctly we've got some way to
determine where things went wrong rather than just ending up with an
empty string? Or are strings the only special case and most other types
will bomb out upon receiving an empty literal.

> The only alternative that I can think of that avoids the latter
> disadvantage is to allow the pqexpbuffer routines to abort on
> out-of-memory (ie, printf(stderr) and exit(1)). This seems pretty
> unpleasant though for functions that are part of libpq's infrastructure.

If there's no way to avoid the abort then this sounds nasty!

> In particular, although we could allow the calling application to
> override such behavior via some sort of callback hook function, it's
> far from clear what it could do instead without risking bizarre
> misbehavior by libpq.

It doesn't seem obvious to me how these callback functions could do
anything useful. They would still need some way of returning an error
to the outside world which would imply some sort of mechanism, like the
one above, to allow this to happen.

I'd be happy writing a patch for this if you want. There appear to be
a couple of thousand references to the PQExpBuffer code, but I can't
imagine needing to touch all of them.

Sam

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Hitoshi Harada 2008-11-26 12:29:42 Re: Windowing Function Patch Review -> Standard Conformance
Previous Message Peter Eisentraut 2008-11-26 12:04:36 Re: Brittleness in regression test setup