Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 9f9d9b5

Browse files
committed
Improve pqexpbuffer.c to use modern vsnprintf implementations efficiently.
When using a C99-compliant vsnprintf, we can use its report of the required buffer size to avoid making multiple loops through the formatting logic. This is similar to the changes recently made in stringinfo.c, but we can't use psprintf.c here because in libpq we don't want to exit() on error. (The behavior pqexpbuffer.c has historically used is to mark the PQExpBuffer as "broken", ie empty, if it runs into any fatal problem.) To avoid duplicating code more than necessary, I refactored printfPQExpBuffer and appendPQExpBuffer to share a subroutine that's very similar to psprintf.c's pvsnprintf in spirit.
1 parent 43fe90f commit 9f9d9b5

File tree

1 file changed

+110
-55
lines changed

1 file changed

+110
-55
lines changed

src/interfaces/libpq/pqexpbuffer.c

+110-55
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,8 @@
88
*
99
* This module is essentially the same as the backend's StringInfo data type,
1010
* but it is intended for use in frontend libpq and client applications.
11-
* Thus, it does not rely on palloc() nor elog().
11+
* Thus, it does not rely on palloc() nor elog(), nor psprintf.c which
12+
* will exit() on error.
1213
*
1314
* It does rely on vsnprintf(); if configure finds that libc doesn't provide
1415
* a usable vsnprintf(), then a copy of our own implementation of it will
@@ -36,6 +37,10 @@
3637
/* All "broken" PQExpBuffers point to this string. */
3738
static const char oom_buffer[1] = "";
3839

40+
static bool
41+
appendPQExpBufferVA(PQExpBuffer str, const char *fmt, va_list args)
42+
__attribute__((format(PG_PRINTF_ATTRIBUTE, 2, 0)));
43+
3944

4045
/*
4146
* markPQExpBufferBroken
@@ -231,45 +236,20 @@ void
231236
printfPQExpBuffer(PQExpBuffer str, const char *fmt,...)
232237
{
233238
va_list args;
234-
size_t avail;
235-
int nprinted;
239+
bool done;
236240

237241
resetPQExpBuffer(str);
238242

239243
if (PQExpBufferBroken(str))
240244
return; /* already failed */
241245

242-
for (;;)
246+
/* Loop in case we have to retry after enlarging the buffer. */
247+
do
243248
{
244-
/*
245-
* Try to format the given string into the available space; but if
246-
* there's hardly any space, don't bother trying, just fall through to
247-
* enlarge the buffer first.
248-
*/
249-
if (str->maxlen > str->len + 16)
250-
{
251-
avail = str->maxlen - str->len - 1;
252-
va_start(args, fmt);
253-
nprinted = vsnprintf(str->data + str->len, avail,
254-
fmt, args);
255-
va_end(args);
256-
257-
/*
258-
* Note: some versions of vsnprintf return the number of chars
259-
* actually stored, but at least one returns -1 on failure. Be
260-
* conservative about believing whether the print worked.
261-
*/
262-
if (nprinted >= 0 && nprinted < (int) avail - 1)
263-
{
264-
/* Success. Note nprinted does not include trailing null. */
265-
str->len += nprinted;
266-
break;
267-
}
268-
}
269-
/* Double the buffer size and try again. */
270-
if (!enlargePQExpBuffer(str, str->maxlen))
271-
return; /* oops, out of memory */
272-
}
249+
va_start(args, fmt);
250+
done = appendPQExpBufferVA(str, fmt, args);
251+
va_end(args);
252+
} while (!done);
273253
}
274254

275255
/*
@@ -284,43 +264,118 @@ void
284264
appendPQExpBuffer(PQExpBuffer str, const char *fmt,...)
285265
{
286266
va_list args;
287-
size_t avail;
288-
int nprinted;
267+
bool done;
289268

290269
if (PQExpBufferBroken(str))
291270
return; /* already failed */
292271

293-
for (;;)
272+
/* Loop in case we have to retry after enlarging the buffer. */
273+
do
274+
{
275+
va_start(args, fmt);
276+
done = appendPQExpBufferVA(str, fmt, args);
277+
va_end(args);
278+
} while (!done);
279+
}
280+
281+
/*
282+
* appendPQExpBufferVA
283+
* Shared guts of printfPQExpBuffer/appendPQExpBuffer.
284+
* Attempt to format data and append it to str. Returns true if done
285+
* (either successful or hard failure), false if need to retry.
286+
*/
287+
static bool
288+
appendPQExpBufferVA(PQExpBuffer str, const char *fmt, va_list args)
289+
{
290+
size_t avail;
291+
size_t needed;
292+
int nprinted;
293+
294+
/*
295+
* Try to format the given string into the available space; but if there's
296+
* hardly any space, don't bother trying, just enlarge the buffer first.
297+
*/
298+
if (str->maxlen > str->len + 16)
294299
{
295300
/*
296-
* Try to format the given string into the available space; but if
297-
* there's hardly any space, don't bother trying, just fall through to
298-
* enlarge the buffer first.
301+
* Note: we intentionally leave one byte unused, as a guard against
302+
* old broken versions of vsnprintf.
299303
*/
300-
if (str->maxlen > str->len + 16)
304+
avail = str->maxlen - str->len - 1;
305+
306+
errno = 0;
307+
308+
nprinted = vsnprintf(str->data + str->len, avail, fmt, args);
309+
310+
/*
311+
* If vsnprintf reports an error other than ENOMEM, fail.
312+
*/
313+
if (nprinted < 0 && errno != 0 && errno != ENOMEM)
301314
{
302-
avail = str->maxlen - str->len - 1;
303-
va_start(args, fmt);
304-
nprinted = vsnprintf(str->data + str->len, avail,
305-
fmt, args);
306-
va_end(args);
315+
markPQExpBufferBroken(str);
316+
return true;
317+
}
307318

319+
/*
320+
* Note: some versions of vsnprintf return the number of chars
321+
* actually stored, not the total space needed as C99 specifies. And
322+
* at least one returns -1 on failure. Be conservative about
323+
* believing whether the print worked.
324+
*/
325+
if (nprinted >= 0 && (size_t) nprinted < avail - 1)
326+
{
327+
/* Success. Note nprinted does not include trailing null. */
328+
str->len += nprinted;
329+
return true;
330+
}
331+
332+
if (nprinted >= 0 && (size_t) nprinted > avail)
333+
{
308334
/*
309-
* Note: some versions of vsnprintf return the number of chars
310-
* actually stored, but at least one returns -1 on failure. Be
311-
* conservative about believing whether the print worked.
335+
* This appears to be a C99-compliant vsnprintf, so believe its
336+
* estimate of the required space. (If it's wrong, the logic will
337+
* still work, but we may loop multiple times.) Note that the
338+
* space needed should be only nprinted+1 bytes, but we'd better
339+
* allocate one more than that so that the test above will succeed
340+
* next time.
341+
*
342+
* In the corner case where the required space just barely
343+
* overflows, fail.
312344
*/
313-
if (nprinted >= 0 && nprinted < (int) avail - 1)
345+
if (nprinted > INT_MAX - 2)
314346
{
315-
/* Success. Note nprinted does not include trailing null. */
316-
str->len += nprinted;
317-
break;
347+
markPQExpBufferBroken(str);
348+
return true;
318349
}
350+
needed = nprinted + 2;
319351
}
320-
/* Double the buffer size and try again. */
321-
if (!enlargePQExpBuffer(str, str->maxlen))
322-
return; /* oops, out of memory */
352+
else
353+
{
354+
/*
355+
* Buffer overrun, and we don't know how much space is needed.
356+
* Estimate twice the previous buffer size, but not more than
357+
* INT_MAX.
358+
*/
359+
if (avail >= INT_MAX / 2)
360+
needed = INT_MAX;
361+
else
362+
needed = avail * 2;
363+
}
364+
}
365+
else
366+
{
367+
/*
368+
* We have to guess at how much to enlarge, since we're skipping the
369+
* formatting work.
370+
*/
371+
needed = 32;
323372
}
373+
374+
/* Increase the buffer size and try again. */
375+
if (!enlargePQExpBuffer(str, needed))
376+
return true; /* oops, out of memory */
377+
378+
return false;
324379
}
325380

326381
/*

0 commit comments

Comments
 (0)