Skip to content

Commit ac4ef63

Browse files
tglsfdctglsfdc
committed
Allow use of "z" flag in our printf calls, and use it where appropriate.
Since C99, it's been standard for printf and friends to accept a "z" size modifier, meaning "whatever size size_t has". Up to now we've generally dealt with printing size_t values by explicitly casting them to unsigned long and using the "l" modifier; but this is really the wrong thing on platforms where pointers are wider than longs (such as Win64). So let's start using "z" instead. To ensure we can do that on all platforms, teach src/port/snprintf.c to understand "z", and add a configure test to force use of that implementation when the platform's version doesn't handle "z". Having done that, modify a bunch of places that were using the unsigned-long hack to use "z" instead. This patch doesn't pretend to have gotten everyplace that could benefit, but it catches many of them. I made an effort in particular to ensure that all uses of the same error message text were updated together, so as not to increase the number of translatable strings. It's possible that this change will result in format-string warnings from pre-C99 compilers. We might have to reconsider if there are any popular compilers that will warn about this; but let's start by seeing what the buildfarm thinks. Andres Freund, with a little additional work by me
1 parent ec8f692 commit ac4ef63

27 files changed

+221
-125
lines changed

‎config/c-library.m4

Lines changed: 47 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -273,16 +273,16 @@ case $pgac_cv_snprintf_long_long_int_format in
273273
esac])# PGAC_FUNC_SNPRINTF_LONG_LONG_INT_FORMAT
274274

275275

276-
# PGAC_FUNC_PRINTF_ARG_CONTROL
276+
# PGAC_FUNC_SNPRINTF_ARG_CONTROL
277277
# ---------------------------------------
278-
# Determine if printf supports %1$ argument selection, e.g. %5$ selects
279-
# the fifth argument after the printf print string.
278+
# Determine if snprintf supports %1$ argument selection, e.g. %5$ selects
279+
# the fifth argument after the printf format string.
280280
# This is not in the C99 standard, but in the Single Unix Specification (SUS).
281281
# It is used in our language translation strings.
282282
#
283-
AC_DEFUN([PGAC_FUNC_PRINTF_ARG_CONTROL],
284-
[AC_MSG_CHECKING([whether printf supports argument control])
285-
AC_CACHE_VAL(pgac_cv_printf_arg_control,
283+
AC_DEFUN([PGAC_FUNC_SNPRINTF_ARG_CONTROL],
284+
[AC_MSG_CHECKING([whether snprintf supports argument control])
285+
AC_CACHE_VAL(pgac_cv_snprintf_arg_control,
286286
[AC_TRY_RUN([#include <stdio.h>
287287
#include <string.h>
288288
@@ -296,12 +296,48 @@ int main()
296296
return 1;
297297
return 0;
298298
}],
299-
[pgac_cv_printf_arg_control=yes],
300-
[pgac_cv_printf_arg_control=no],
301-
[pgac_cv_printf_arg_control=cross])
299+
[pgac_cv_snprintf_arg_control=yes],
300+
[pgac_cv_snprintf_arg_control=no],
301+
[pgac_cv_snprintf_arg_control=cross])
302302
])dnl AC_CACHE_VAL
303-
AC_MSG_RESULT([$pgac_cv_printf_arg_control])
304-
])# PGAC_FUNC_PRINTF_ARG_CONTROL
303+
AC_MSG_RESULT([$pgac_cv_snprintf_arg_control])
304+
])# PGAC_FUNC_SNPRINTF_ARG_CONTROL
305+
306+
# PGAC_FUNC_SNPRINTF_SIZE_T_SUPPORT
307+
# ---------------------------------------
308+
# Determine if snprintf supports the z length modifier for printing
309+
# size_t-sized variables. That's supported by C99 and POSIX but not
310+
# all platforms play ball, so we must test whether it's working.
311+
#
312+
AC_DEFUN([PGAC_FUNC_SNPRINTF_SIZE_T_SUPPORT],
313+
[AC_MSG_CHECKING([whether snprintf supports the %z modifier])
314+
AC_CACHE_VAL(pgac_cv_snprintf_size_t_support,
315+
[AC_TRY_RUN([#include <stdio.h>
316+
#include <string.h>
317+
318+
int main()
319+
{
320+
char bufz[100];
321+
char buf64[100];
322+
323+
/*
324+
* Print the largest unsigned number fitting in a size_t using both %zu
325+
* and the previously-determined format for 64-bit integers. Note that
326+
* we don't run this code unless we know snprintf handles 64-bit ints.
327+
*/
328+
bufz[0] = '\0'; /* in case snprintf fails to emit anything */
329+
snprintf(bufz, sizeof(bufz), "%zu", ~((size_t) 0));
330+
snprintf(buf64, sizeof(buf64), UINT64_FORMAT, (PG_INT64_TYPE) ~((size_t) 0));
331+
if (strcmp(bufz, buf64) != 0)
332+
return 1;
333+
return 0;
334+
}],
335+
[pgac_cv_snprintf_size_t_support=yes],
336+
[pgac_cv_snprintf_size_t_support=no],
337+
[pgac_cv_snprintf_size_t_support=cross])
338+
])dnl AC_CACHE_VAL
339+
AC_MSG_RESULT([$pgac_cv_snprintf_size_t_support])
340+
])# PGAC_FUNC_SNPRINTF_SIZE_T_SUPPORT
305341

306342

307343
# PGAC_TYPE_LOCALE_T

‎configure

Lines changed: 61 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -12698,13 +12698,13 @@ fi
1269812698
# Force use of our snprintf if system's doesn't do arg control
1269912699
# See comment above at snprintf test for details.
1270012700
if test "$enable_nls" = yes -a "$pgac_need_repl_snprintf" = no; then
12701-
{ $as_echo "$as_me:${as_lineno-$LINENO}: checking whether printf supports argument control" >&5
12702-
$as_echo_n "checking whether printf supports argument control... " >&6; }
12703-
if ${pgac_cv_printf_arg_control+:} false; then :
12701+
{ $as_echo "$as_me:${as_lineno-$LINENO}: checking whether snprintf supports argument control" >&5
12702+
$as_echo_n "checking whether snprintf supports argument control... " >&6; }
12703+
if ${pgac_cv_snprintf_arg_control+:} false; then :
1270412704
$as_echo_n "(cached) " >&6
1270512705
else
1270612706
if test "$cross_compiling" = yes; then :
12707-
pgac_cv_printf_arg_control=cross
12707+
pgac_cv_snprintf_arg_control=cross
1270812708
else
1270912709
cat confdefs.h - <<_ACEOF >conftest.$ac_ext
1271012710
/* end confdefs.h. */
@@ -12723,20 +12723,20 @@ int main()
1272312723
}
1272412724
_ACEOF
1272512725
if ac_fn_c_try_run "$LINENO"; then :
12726-
pgac_cv_printf_arg_control=yes
12726+
pgac_cv_snprintf_arg_control=yes
1272712727
else
12728-
pgac_cv_printf_arg_control=no
12728+
pgac_cv_snprintf_arg_control=no
1272912729
fi
1273012730
rm -f core *.core core.conftest.* gmon.out bb.out conftest$ac_exeext \
1273112731
conftest.$ac_objext conftest.beam conftest.$ac_ext
1273212732
fi
1273312733

1273412734

1273512735
fi
12736-
{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $pgac_cv_printf_arg_control" >&5
12737-
$as_echo "$pgac_cv_printf_arg_control" >&6; }
12736+
{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $pgac_cv_snprintf_arg_control" >&5
12737+
$as_echo "$pgac_cv_snprintf_arg_control" >&6; }
1273812738

12739-
if test $pgac_cv_printf_arg_control != yes ; then
12739+
if test $pgac_cv_snprintf_arg_control != yes ; then
1274012740
pgac_need_repl_snprintf=yes
1274112741
fi
1274212742
fi
@@ -13036,6 +13036,58 @@ cat >>confdefs.h <<_ACEOF
1303613036
_ACEOF
1303713037

1303813038

13039+
# Also force use of our snprintf if the system's doesn't support the %z flag.
13040+
if test "$pgac_need_repl_snprintf" = no; then
13041+
{ $as_echo "$as_me:${as_lineno-$LINENO}: checking whether snprintf supports the %z modifier" >&5
13042+
$as_echo_n "checking whether snprintf supports the %z modifier... " >&6; }
13043+
if ${pgac_cv_snprintf_size_t_support+:} false; then :
13044+
$as_echo_n "(cached) " >&6
13045+
else
13046+
if test "$cross_compiling" = yes; then :
13047+
pgac_cv_snprintf_size_t_support=cross
13048+
else
13049+
cat confdefs.h - <<_ACEOF >conftest.$ac_ext
13050+
/* end confdefs.h. */
13051+
#include <stdio.h>
13052+
#include <string.h>
13053+
13054+
int main()
13055+
{
13056+
char bufz[100];
13057+
char buf64[100];
13058+
13059+
/*
13060+
* Print the largest unsigned number fitting in a size_t using both %zu
13061+
* and the previously-determined format for 64-bit integers. Note that
13062+
* we don't run this code unless we know snprintf handles 64-bit ints.
13063+
*/
13064+
bufz[0] = '\0'; /* in case snprintf fails to emit anything */
13065+
snprintf(bufz, sizeof(bufz), "%zu", ~((size_t) 0));
13066+
snprintf(buf64, sizeof(buf64), UINT64_FORMAT, (PG_INT64_TYPE) ~((size_t) 0));
13067+
if (strcmp(bufz, buf64) != 0)
13068+
return 1;
13069+
return 0;
13070+
}
13071+
_ACEOF
13072+
if ac_fn_c_try_run "$LINENO"; then :
13073+
pgac_cv_snprintf_size_t_support=yes
13074+
else
13075+
pgac_cv_snprintf_size_t_support=no
13076+
fi
13077+
rm -f core *.core core.conftest.* gmon.out bb.out conftest$ac_exeext \
13078+
conftest.$ac_objext conftest.beam conftest.$ac_ext
13079+
fi
13080+
13081+
13082+
fi
13083+
{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $pgac_cv_snprintf_size_t_support" >&5
13084+
$as_echo "$pgac_cv_snprintf_size_t_support" >&6; }
13085+
13086+
if test "$pgac_cv_snprintf_size_t_support" != yes; then
13087+
pgac_need_repl_snprintf=yes
13088+
fi
13089+
fi
13090+
1303913091
# Now we have checked all the reasons to replace snprintf
1304013092
if test $pgac_need_repl_snprintf = yes; then
1304113093

‎configure.in

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1533,8 +1533,8 @@ for the exact reason.]])],
15331533
# Force use of our snprintf if system's doesn't do arg control
15341534
# See comment above at snprintf test for details.
15351535
if test "$enable_nls" = yes -a "$pgac_need_repl_snprintf" = no; then
1536-
PGAC_FUNC_PRINTF_ARG_CONTROL
1537-
if test $pgac_cv_printf_arg_control != yes ; then
1536+
PGAC_FUNC_SNPRINTF_ARG_CONTROL
1537+
if test $pgac_cv_snprintf_arg_control != yes ; then
15381538
pgac_need_repl_snprintf=yes
15391539
fi
15401540
fi
@@ -1617,6 +1617,14 @@ AC_DEFINE_UNQUOTED(INT64_FORMAT, $INT64_FORMAT,
16171617
AC_DEFINE_UNQUOTED(UINT64_FORMAT, $UINT64_FORMAT,
16181618
[Define to the appropriate snprintf format for unsigned 64-bit ints.])
16191619

1620+
# Also force use of our snprintf if the system's doesn't support the %z flag.
1621+
if test "$pgac_need_repl_snprintf" = no; then
1622+
PGAC_FUNC_SNPRINTF_SIZE_T_SUPPORT
1623+
if test "$pgac_cv_snprintf_size_t_support" != yes; then
1624+
pgac_need_repl_snprintf=yes
1625+
fi
1626+
fi
1627+
16201628
# Now we have checked all the reasons to replace snprintf
16211629
if test $pgac_need_repl_snprintf = yes; then
16221630
AC_DEFINE(USE_REPL_SNPRINTF, 1, [Use replacement snprintf() functions.])

‎src/backend/access/common/indextuple.c

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -165,9 +165,8 @@ index_form_tuple(TupleDesc tupleDescriptor,
165165
if ((size & INDEX_SIZE_MASK) != size)
166166
ereport(ERROR,
167167
(errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
168-
errmsg("index row requires %lu bytes, maximum size is %lu",
169-
(unsigned long) size,
170-
(unsigned long) INDEX_SIZE_MASK)));
168+
errmsg("index row requires %zu bytes, maximum size is %zu",
169+
size, (Size) INDEX_SIZE_MASK)));
171170

172171
infomask |= size;
173172

‎src/backend/access/gin/ginentrypage.c

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -105,9 +105,8 @@ GinFormTuple(GinState *ginstate,
105105
if (errorTooBig)
106106
ereport(ERROR,
107107
(errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
108-
errmsg("index row size %lu exceeds maximum %lu for index \"%s\"",
109-
(unsigned long) newsize,
110-
(unsigned long) GinMaxItemSize,
108+
errmsg("index row size %zu exceeds maximum %zu for index \"%s\"",
109+
(Size) newsize, (Size) GinMaxItemSize,
111110
RelationGetRelationName(ginstate->index))));
112111
pfree(itup);
113112
return NULL;

‎src/backend/access/hash/hashinsert.c

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -65,9 +65,8 @@ _hash_doinsert(Relation rel, IndexTuple itup)
6565
if (itemsz > HashMaxItemSize((Page) metap))
6666
ereport(ERROR,
6767
(errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
68-
errmsg("index row size %lu exceeds hash maximum %lu",
69-
(unsigned long) itemsz,
70-
(unsigned long) HashMaxItemSize((Page) metap)),
68+
errmsg("index row size %zu exceeds hash maximum %zu",
69+
itemsz, HashMaxItemSize((Page) metap)),
7170
errhint("Values larger than a buffer page cannot be indexed.")));
7271

7372
/*

‎src/backend/access/heap/hio.c

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -237,9 +237,8 @@ RelationGetBufferForTuple(Relation relation, Size len,
237237
if (len > MaxHeapTupleSize)
238238
ereport(ERROR,
239239
(errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
240-
errmsg("row is too big: size %lu, maximum size %lu",
241-
(unsigned long) len,
242-
(unsigned long) MaxHeapTupleSize)));
240+
errmsg("row is too big: size %zu, maximum size %zu",
241+
len, MaxHeapTupleSize)));
243242

244243
/* Compute desired extra freespace due to fillfactor option */
245244
saveFreeSpace = RelationGetTargetPageFreeSpace(relation,
@@ -477,7 +476,7 @@ RelationGetBufferForTuple(Relation relation, Size len,
477476
if (len > PageGetHeapFreeSpace(page))
478477
{
479478
/* We should not get here given the test at the top */
480-
elog(PANIC, "tuple is too big: size %lu", (unsigned long) len);
479+
elog(PANIC, "tuple is too big: size %zu", len);
481480
}
482481

483482
/*

‎src/backend/access/heap/rewriteheap.c

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -601,9 +601,8 @@ raw_heap_insert(RewriteState state, HeapTuple tup)
601601
if (len > MaxHeapTupleSize)
602602
ereport(ERROR,
603603
(errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
604-
errmsg("row is too big: size %lu, maximum size %lu",
605-
(unsigned long) len,
606-
(unsigned long) MaxHeapTupleSize)));
604+
errmsg("row is too big: size %zu, maximum size %zu",
605+
len, MaxHeapTupleSize)));
607606

608607
/* Compute desired extra freespace due to fillfactor option */
609608
saveFreeSpace = RelationGetTargetPageFreeSpace(state->rs_new_rel,

‎src/backend/access/nbtree/nbtinsert.c

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -537,9 +537,8 @@ _bt_findinsertloc(Relation rel,
537537
if (itemsz > BTMaxItemSize(page))
538538
ereport(ERROR,
539539
(errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
540-
errmsg("index row size %lu exceeds maximum %lu for index \"%s\"",
541-
(unsigned long) itemsz,
542-
(unsigned long) BTMaxItemSize(page),
540+
errmsg("index row size %zu exceeds maximum %zu for index \"%s\"",
541+
itemsz, BTMaxItemSize(page),
543542
RelationGetRelationName(rel)),
544543
errhint("Values larger than 1/3 of a buffer page cannot be indexed.\n"
545544
"Consider a function index of an MD5 hash of the value, "

‎src/backend/access/nbtree/nbtsort.c

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -482,9 +482,8 @@ _bt_buildadd(BTWriteState *wstate, BTPageState *state, IndexTuple itup)
482482
if (itupsz > BTMaxItemSize(npage))
483483
ereport(ERROR,
484484
(errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
485-
errmsg("index row size %lu exceeds maximum %lu for index \"%s\"",
486-
(unsigned long) itupsz,
487-
(unsigned long) BTMaxItemSize(npage),
485+
errmsg("index row size %zu exceeds maximum %zu for index \"%s\"",
486+
itupsz, BTMaxItemSize(npage),
488487
RelationGetRelationName(wstate->index)),
489488
errhint("Values larger than 1/3 of a buffer page cannot be indexed.\n"
490489
"Consider a function index of an MD5 hash of the value, "

‎src/backend/access/spgist/spgdoinsert.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1885,9 +1885,9 @@ spgdoinsert(Relation index, SpGistState *state,
18851885
if (leafSize > SPGIST_PAGE_CAPACITY && !state->config.longValuesOK)
18861886
ereport(ERROR,
18871887
(errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
1888-
errmsg("index row size %lu exceeds maximum %lu for index \"%s\"",
1889-
(unsigned long) (leafSize - sizeof(ItemIdData)),
1890-
(unsigned long) (SPGIST_PAGE_CAPACITY - sizeof(ItemIdData)),
1888+
errmsg("index row size %zu exceeds maximum %zu for index \"%s\"",
1889+
leafSize - sizeof(ItemIdData),
1890+
SPGIST_PAGE_CAPACITY - sizeof(ItemIdData),
18911891
RelationGetRelationName(index)),
18921892
errhint("Values larger than a buffer page cannot be indexed.")));
18931893

‎src/backend/access/spgist/spgutils.c

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -602,9 +602,8 @@ spgFormNodeTuple(SpGistState *state, Datum label, bool isnull)
602602
if ((size & INDEX_SIZE_MASK) != size)
603603
ereport(ERROR,
604604
(errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
605-
errmsg("index row requires %lu bytes, maximum size is %lu",
606-
(unsigned long) size,
607-
(unsigned long) INDEX_SIZE_MASK)));
605+
errmsg("index row requires %zu bytes, maximum size is %zu",
606+
(Size) size, (Size) INDEX_SIZE_MASK)));
608607

609608
tup = (SpGistNodeTuple) palloc0(size);
610609

@@ -661,9 +660,9 @@ spgFormInnerTuple(SpGistState *state, bool hasPrefix, Datum prefix,
661660
if (size > SPGIST_PAGE_CAPACITY - sizeof(ItemIdData))
662661
ereport(ERROR,
663662
(errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
664-
errmsg("SP-GiST inner tuple size %lu exceeds maximum %lu",
665-
(unsigned long) size,
666-
(unsigned long) (SPGIST_PAGE_CAPACITY - sizeof(ItemIdData))),
663+
errmsg("SP-GiST inner tuple size %zu exceeds maximum %zu",
664+
(Size) size,
665+
SPGIST_PAGE_CAPACITY - sizeof(ItemIdData)),
667666
errhint("Values larger than a buffer page cannot be indexed.")));
668667

669668
/*

‎src/backend/access/transam/xlog.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2738,9 +2738,9 @@ XLogWrite(XLogwrtRqst WriteRqst, bool flexible)
27382738
ereport(PANIC,
27392739
(errcode_for_file_access(),
27402740
errmsg("could not write to log file %s "
2741-
"at offset %u, length %lu: %m",
2741+
"at offset %u, length %zu: %m",
27422742
XLogFileNameP(ThisTimeLineID, openLogSegNo),
2743-
openLogOff, (unsigned long) nbytes)));
2743+
openLogOff, nbytes)));
27442744
}
27452745
nleft -= written;
27462746
from += written;

‎src/backend/nodes/readfuncs.c

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1439,15 +1439,13 @@ readDatum(bool typbyval)
14391439

14401440
token = pg_strtok(&tokenLength); /* read the '[' */
14411441
if (token == NULL || token[0] != '[')
1442-
elog(ERROR, "expected \"[\" to start datum, but got \"%s\"; length = %lu",
1443-
token ? (const char *) token : "[NULL]",
1444-
(unsigned long) length);
1442+
elog(ERROR, "expected \"[\" to start datum, but got \"%s\"; length = %zu",
1443+
token ? (const char *) token : "[NULL]", length);
14451444

14461445
if (typbyval)
14471446
{
14481447
if (length > (Size) sizeof(Datum))
1449-
elog(ERROR, "byval datum but length = %lu",
1450-
(unsigned long) length);
1448+
elog(ERROR, "byval datum but length = %zu", length);
14511449
res = (Datum) 0;
14521450
s = (char *) (&res);
14531451
for (i = 0; i < (Size) sizeof(Datum); i++)
@@ -1471,9 +1469,8 @@ readDatum(bool typbyval)
14711469

14721470
token = pg_strtok(&tokenLength); /* read the ']' */
14731471
if (token == NULL || token[0] != ']')
1474-
elog(ERROR, "expected \"]\" to end datum, but got \"%s\"; length = %lu",
1475-
token ? (const char *) token : "[NULL]",
1476-
(unsigned long) length);
1472+
elog(ERROR, "expected \"]\" to end datum, but got \"%s\"; length = %zu",
1473+
token ? (const char *) token : "[NULL]", length);
14771474

14781475
return res;
14791476
}

0 commit comments

Comments
 (0)