Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commite1d19c9

Browse files
committed
Require a C99-compliant snprintf(), and remove related workarounds.
Since our substitute snprintf now returns a C99-compliant result,there's no need anymore to have complicated code to cope with pre-C99behavior. We can just make configure substitute snprintf.c if it findsthat the system snprintf() is pre-C99. (Note: I do not believe thatthere are any platforms where this test will trigger that weren'talready being rejected due to our other C99-ish feature requirements forsnprintf. But let's add the check for paranoia's sake.) Then, simplifythe call sites that had logic to cope with the pre-C99 definition.I also dropped some stuff that was being paranoid about the possibilityof snprintf overrunning the given buffer. The only reports we've everheard of that being a problem were for Solaris 7, which is long dead,and we've sure not heard any reports of these assertions triggering ina long time. So let's drop that complexity too.Likewise, drop some code that wasn't trusting snprintf to set errnowhen it returns -1. That would be not-per-spec, and again there'sno real reason to believe it is a live issue, especially not forsnprintfs that pass all of configure's feature checks.Discussion:https://postgr.es/m/17245.1534289329@sss.pgh.pa.us
1 parent1eb9221 commite1d19c9

File tree

6 files changed

+125
-123
lines changed

6 files changed

+125
-123
lines changed

‎config/c-library.m4

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -238,6 +238,33 @@ int main()
238238
AC_MSG_RESULT([$pgac_cv_snprintf_size_t_support])
239239
])# PGAC_FUNC_SNPRINTF_SIZE_T_SUPPORT
240240

241+
# PGAC_FUNC_SNPRINTF_C99_RESULT
242+
# -----------------------------
243+
# Determine whether snprintf returns the desired buffer length when
244+
# it overruns the actual buffer length. That's required by C99 and POSIX
245+
# but ancient platforms don't behave that way, so we must test.
246+
#
247+
AC_DEFUN([PGAC_FUNC_SNPRINTF_C99_RESULT],
248+
[AC_MSG_CHECKING([whether snprintf reports buffer overrun per C99])
249+
AC_CACHE_VAL(pgac_cv_snprintf_c99_result,
250+
[AC_RUN_IFELSE([AC_LANG_SOURCE([[#include <stdio.h>
251+
#include <string.h>
252+
253+
int main()
254+
{
255+
char buf[10];
256+
257+
if (snprintf(buf, sizeof(buf), "12345678901234567890") != 20)
258+
return 1;
259+
return 0;
260+
}]])],
261+
[pgac_cv_snprintf_c99_result=yes],
262+
[pgac_cv_snprintf_c99_result=no],
263+
[pgac_cv_snprintf_c99_result=cross])
264+
])dnl AC_CACHE_VAL
265+
AC_MSG_RESULT([$pgac_cv_snprintf_c99_result])
266+
])# PGAC_FUNC_SNPRINTF_C99_RESULT
267+
241268

242269
# PGAC_TYPE_LOCALE_T
243270
# ------------------

‎configure

Lines changed: 45 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16142,7 +16142,7 @@ fi
1614216142
# Run tests below here
1614316143
# --------------------
1614416144

16145-
#Forceuse of our snprintf if system's doesn't do arg control
16145+
#For NLS, forceuse of our snprintf if system's doesn't do arg control.
1614616146
# See comment above at snprintf test for details.
1614716147
if test "$enable_nls" = yes -a "$pgac_need_repl_snprintf" = no; then
1614816148
{ $as_echo "$as_me:${as_lineno-$LINENO}: checking whether snprintf supports argument control" >&5
@@ -16436,6 +16436,50 @@ $as_echo "$pgac_cv_snprintf_size_t_support" >&6; }
1643616436
fi
1643716437
fi
1643816438

16439+
# Force use of our snprintf if the system's doesn't handle buffer overrun
16440+
# as specified by C99.
16441+
if test "$pgac_need_repl_snprintf" = no; then
16442+
{ $as_echo "$as_me:${as_lineno-$LINENO}: checking whether snprintf reports buffer overrun per C99" >&5
16443+
$as_echo_n "checking whether snprintf reports buffer overrun per C99... " >&6; }
16444+
if ${pgac_cv_snprintf_c99_result+:} false; then :
16445+
$as_echo_n "(cached) " >&6
16446+
else
16447+
if test "$cross_compiling" = yes; then :
16448+
pgac_cv_snprintf_c99_result=cross
16449+
else
16450+
cat confdefs.h - <<_ACEOF >conftest.$ac_ext
16451+
/* end confdefs.h. */
16452+
#include <stdio.h>
16453+
#include <string.h>
16454+
16455+
int main()
16456+
{
16457+
char buf[10];
16458+
16459+
if (snprintf(buf, sizeof(buf), "12345678901234567890") != 20)
16460+
return 1;
16461+
return 0;
16462+
}
16463+
_ACEOF
16464+
if ac_fn_c_try_run "$LINENO"; then :
16465+
pgac_cv_snprintf_c99_result=yes
16466+
else
16467+
pgac_cv_snprintf_c99_result=no
16468+
fi
16469+
rm -f core *.core core.conftest.* gmon.out bb.out conftest$ac_exeext \
16470+
conftest.$ac_objext conftest.beam conftest.$ac_ext
16471+
fi
16472+
16473+
16474+
fi
16475+
{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $pgac_cv_snprintf_c99_result" >&5
16476+
$as_echo "$pgac_cv_snprintf_c99_result" >&6; }
16477+
16478+
if test "$pgac_cv_snprintf_c99_result" != yes; then
16479+
pgac_need_repl_snprintf=yes
16480+
fi
16481+
fi
16482+
1643916483
# Now we have checked all the reasons to replace snprintf
1644016484
if test $pgac_need_repl_snprintf = yes; then
1644116485

‎configure.in

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1806,7 +1806,7 @@ for the exact reason.]])],
18061806
# Run tests below here
18071807
# --------------------
18081808

1809-
#Forceuse of our snprintf if system's doesn't do arg control
1809+
#For NLS, forceuse of our snprintf if system's doesn't do arg control.
18101810
# See comment above at snprintf test for details.
18111811
if test "$enable_nls" = yes -a "$pgac_need_repl_snprintf" = no; then
18121812
PGAC_FUNC_SNPRINTF_ARG_CONTROL
@@ -1858,6 +1858,15 @@ if test "$pgac_need_repl_snprintf" = no; then
18581858
fi
18591859
fi
18601860

1861+
# Force use of our snprintf if the system's doesn't handle buffer overrun
1862+
# as specified by C99.
1863+
if test "$pgac_need_repl_snprintf" = no; then
1864+
PGAC_FUNC_SNPRINTF_C99_RESULT
1865+
if test "$pgac_cv_snprintf_c99_result" != yes; then
1866+
pgac_need_repl_snprintf=yes
1867+
fi
1868+
fi
1869+
18611870
# Now we have checked all the reasons to replace snprintf
18621871
if test $pgac_need_repl_snprintf = yes; then
18631872
AC_DEFINE(USE_REPL_SNPRINTF, 1, [Use replacement snprintf() functions.])

‎src/backend/utils/misc/guc.c

Lines changed: 8 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -9441,26 +9441,19 @@ do_serialize(char **destptr, Size *maxbytes, const char *fmt,...)
94419441
if (*maxbytes <=0)
94429442
elog(ERROR,"not enough space to serialize GUC state");
94439443

9444-
errno=0;
9445-
94469444
va_start(vargs,fmt);
94479445
n=vsnprintf(*destptr,*maxbytes,fmt,vargs);
94489446
va_end(vargs);
94499447

9450-
/*
9451-
* Cater to portability hazards in the vsnprintf() return value just like
9452-
* appendPQExpBufferVA() does. Note that this requires an extra byte of
9453-
* slack at the end of the buffer. Since serialize_variable() ends with a
9454-
* do_serialize_binary() rather than a do_serialize(), we'll always have
9455-
* that slack; estimate_variable_size() need not add a byte for it.
9456-
*/
9457-
if (n<0||n >=*maxbytes-1)
9448+
if (n<0)
94589449
{
9459-
if (n<0&&errno!=0&&errno!=ENOMEM)
9460-
/* Shouldn't happen. Better show errno description. */
9461-
elog(ERROR,"vsnprintf failed: %m");
9462-
else
9463-
elog(ERROR,"not enough space to serialize GUC state");
9450+
/* Shouldn't happen. Better show errno description. */
9451+
elog(ERROR,"vsnprintf failed: %m");
9452+
}
9453+
if (n >=*maxbytes)
9454+
{
9455+
/* This shouldn't happen either, really. */
9456+
elog(ERROR,"not enough space to serialize GUC state");
94649457
}
94659458

94669459
/* Shift the destptr ahead of the null terminator */

‎src/common/psprintf.c

Lines changed: 14 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ psprintf(const char *fmt,...)
7777
* pvsnprintf
7878
*
7979
* Attempt to format text data under the control of fmt (an sprintf-style
80-
* format string) and insert it into buf (which has length len, len > 0).
80+
* format string) and insert it into buf (which has length len).
8181
*
8282
* If successful, return the number of bytes emitted, not counting the
8383
* trailing zero byte. This will always be strictly less than len.
@@ -89,14 +89,11 @@ psprintf(const char *fmt,...)
8989
* Other error cases do not return, but exit via elog(ERROR) or exit().
9090
* Hence, this shouldn't be used inside libpq.
9191
*
92-
* This function exists mainly to centralize our workarounds for
93-
* non-C99-compliant vsnprintf implementations. Generally, any call that
94-
* pays any attention to the return value should go through here rather
95-
* than calling snprintf or vsnprintf directly.
96-
*
9792
* Note that the semantics of the return value are not exactly C99's.
9893
* First, we don't promise that the estimated buffer size is exactly right;
9994
* callers must be prepared to loop multiple times to get the right size.
95+
* (Given a C99-compliant vsnprintf, that won't happen, but it is rumored
96+
* that some implementations don't always return the same value ...)
10097
* Second, we return the recommended buffer size, not one less than that;
10198
* this lets overflow concerns be handled here rather than in the callers.
10299
*/
@@ -105,28 +102,10 @@ pvsnprintf(char *buf, size_t len, const char *fmt, va_list args)
105102
{
106103
intnprinted;
107104

108-
Assert(len>0);
109-
110-
errno=0;
111-
112-
/*
113-
* Assert check here is to catch buggy vsnprintf that overruns the
114-
* specified buffer length. Solaris 7 in 64-bit mode is an example of a
115-
* platform with such a bug.
116-
*/
117-
#ifdefUSE_ASSERT_CHECKING
118-
buf[len-1]='\0';
119-
#endif
120-
121105
nprinted=vsnprintf(buf,len,fmt,args);
122106

123-
Assert(buf[len-1]=='\0');
124-
125-
/*
126-
* If vsnprintf reports an error other than ENOMEM, fail. The possible
127-
* causes of this are not user-facing errors, so elog should be enough.
128-
*/
129-
if (nprinted<0&&errno!=0&&errno!=ENOMEM)
107+
/* We assume failure means the fmt is bogus, hence hard failure is OK */
108+
if (unlikely(nprinted<0))
130109
{
131110
#ifndefFRONTEND
132111
elog(ERROR,"vsnprintf failed: %m");
@@ -136,42 +115,21 @@ pvsnprintf(char *buf, size_t len, const char *fmt, va_list args)
136115
#endif
137116
}
138117

139-
/*
140-
* Note: some versions of vsnprintf return the number of chars actually
141-
* stored, not the total space needed as C99 specifies. And at least one
142-
* returns -1 on failure. Be conservative about believing whether the
143-
* print worked.
144-
*/
145-
if (nprinted >=0&& (size_t)nprinted<len-1)
118+
if ((size_t)nprinted<len)
146119
{
147120
/* Success. Note nprinted does not include trailing null. */
148121
return (size_t)nprinted;
149122
}
150123

151-
if (nprinted >=0&& (size_t)nprinted>len)
152-
{
153-
/*
154-
* This appears to be a C99-compliant vsnprintf, so believe its
155-
* estimate of the required space. (If it's wrong, the logic will
156-
* still work, but we may loop multiple times.) Note that the space
157-
* needed should be only nprinted+1 bytes, but we'd better allocate
158-
* one more than that so that the test above will succeed next time.
159-
*
160-
* In the corner case where the required space just barely overflows,
161-
* fall through so that we'll error out below (possibly after
162-
* looping).
163-
*/
164-
if ((size_t)nprinted <=MaxAllocSize-2)
165-
returnnprinted+2;
166-
}
167-
168124
/*
169-
* Buffer overrun, and we don't know how much space is needed. Estimate
170-
* twice the previous buffer size, but not more than MaxAllocSize; if we
171-
* are already at MaxAllocSize, choke. Note we use this palloc-oriented
172-
* overflow limit even when in frontend.
125+
* We assume a C99-compliant vsnprintf, so believe its estimate of the
126+
* required space, and add one for the trailing null. (If it's wrong, the
127+
* logic will still work, but we may loop multiple times.)
128+
*
129+
* Choke if the required space would exceed MaxAllocSize. Note we use
130+
* this palloc-oriented overflow limit even when in frontend.
173131
*/
174-
if (len >=MaxAllocSize)
132+
if (unlikely((size_t)nprinted>MaxAllocSize-1))
175133
{
176134
#ifndefFRONTEND
177135
ereport(ERROR,
@@ -183,8 +141,5 @@ pvsnprintf(char *buf, size_t len, const char *fmt, va_list args)
183141
#endif
184142
}
185143

186-
if (len >=MaxAllocSize /2)
187-
returnMaxAllocSize;
188-
189-
returnlen*2;
144+
returnnprinted+1;
190145
}

‎src/interfaces/libpq/pqexpbuffer.c

Lines changed: 21 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -295,76 +295,50 @@ appendPQExpBufferVA(PQExpBuffer str, const char *fmt, va_list args)
295295
*/
296296
if (str->maxlen>str->len+16)
297297
{
298-
/*
299-
* Note: we intentionally leave one byte unused, as a guard against
300-
* old broken versions of vsnprintf.
301-
*/
302-
avail=str->maxlen-str->len-1;
303-
304-
errno=0;
298+
avail=str->maxlen-str->len;
305299

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

308302
/*
309-
* If vsnprintf reports an error other than ENOMEM, fail.
303+
* If vsnprintf reports an error, fail (we assume this means there's
304+
* something wrong with the format string).
310305
*/
311-
if (nprinted<0&&errno!=0&&errno!=ENOMEM)
306+
if (unlikely(nprinted<0))
312307
{
313308
markPQExpBufferBroken(str);
314309
return true;
315310
}
316311

317-
/*
318-
* Note: some versions of vsnprintf return the number of chars
319-
* actually stored, not the total space needed as C99 specifies. And
320-
* at least one returns -1 on failure. Be conservative about
321-
* believing whether the print worked.
322-
*/
323-
if (nprinted >=0&& (size_t)nprinted<avail-1)
312+
if ((size_t)nprinted<avail)
324313
{
325314
/* Success. Note nprinted does not include trailing null. */
326315
str->len+=nprinted;
327316
return true;
328317
}
329318

330-
if (nprinted >=0&& (size_t)nprinted>avail)
331-
{
332-
/*
333-
* This appears to be a C99-compliant vsnprintf, so believe its
334-
* estimate of the required space. (If it's wrong, the logic will
335-
* still work, but we may loop multiple times.) Note that the
336-
* space needed should be only nprinted+1 bytes, but we'd better
337-
* allocate one more than that so that the test above will succeed
338-
* next time.
339-
*
340-
* In the corner case where the required space just barely
341-
* overflows, fail.
342-
*/
343-
if (nprinted>INT_MAX-2)
344-
{
345-
markPQExpBufferBroken(str);
346-
return true;
347-
}
348-
needed=nprinted+2;
349-
}
350-
else
319+
/*
320+
* We assume a C99-compliant vsnprintf, so believe its estimate of the
321+
* required space, and add one for the trailing null. (If it's wrong,
322+
* the logic will still work, but we may loop multiple times.)
323+
*
324+
* Choke if the required space would exceed INT_MAX, since str->maxlen
325+
* can't represent more than that.
326+
*/
327+
if (unlikely(nprinted>INT_MAX-1))
351328
{
352-
/*
353-
* Buffer overrun, and we don't know how much space is needed.
354-
* Estimate twice the previous buffer size, but not more than
355-
* INT_MAX.
356-
*/
357-
if (avail >=INT_MAX /2)
358-
needed=INT_MAX;
359-
else
360-
needed=avail*2;
329+
markPQExpBufferBroken(str);
330+
return true;
361331
}
332+
needed=nprinted+1;
362333
}
363334
else
364335
{
365336
/*
366337
* We have to guess at how much to enlarge, since we're skipping the
367-
* formatting work.
338+
* formatting work. Fortunately, because of enlargePQExpBuffer's
339+
* preference for power-of-2 sizes, this number isn't very sensitive;
340+
* the net effect is that we'll double the buffer size before trying
341+
* to run vsnprintf, which seems sensible.
368342
*/
369343
needed=32;
370344
}

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp