From 3a4b891964a531aa7d242a48fcd9e41379863ead Mon Sep 17 00:00:00 2001 From: Peter Eisentraut Date: Thu, 15 Mar 2018 11:10:41 -0400 Subject: [PATCH] Fix more format truncation issues Fix the warnings created by the compiler warning options -Wformat-overflow=2 -Wformat-truncation=2, supported since GCC 7. This is a more aggressive variant of the fixes in 6275f5d28a1577563f53f2171689d4f890a46881, which GCC 7 warned about by default. The issues are all harmless, but some dubious coding patterns are cleaned up. One issue that is of external interest is that BGW_MAXLEN is increased from 64 to 96. Apparently, the old value would cause the bgw_name of logical replication workers to be truncated in some circumstances. But this doesn't actually add those warning options. It appears that the warnings depend a bit on compilation and optimization options, so it would be annoying to have to keep up with that. This is more of a once-in-a-while cleanup. Reviewed-by: Michael Paquier --- contrib/pgstattuple/pgstattuple.c | 2 +- src/backend/commands/explain.c | 5 ++-- src/backend/libpq/be-secure-openssl.c | 2 +- src/backend/utils/adt/dbsize.c | 2 +- src/backend/utils/adt/float.c | 24 +++++++---------- src/backend/utils/adt/formatting.c | 33 +++++++----------------- src/backend/utils/misc/guc.c | 4 +-- src/bin/initdb/initdb.c | 6 ++--- src/bin/pg_dump/pg_backup_archiver.c | 2 +- src/bin/pg_dump/pg_backup_tar.c | 2 +- src/bin/pgbench/pgbench.c | 4 +-- src/include/postmaster/bgworker.h | 2 +- src/interfaces/libpq/fe-secure-openssl.c | 2 +- src/pl/tcl/pltcl.c | 2 +- 14 files changed, 38 insertions(+), 54 deletions(-) diff --git a/contrib/pgstattuple/pgstattuple.c b/contrib/pgstattuple/pgstattuple.c index 7ca1bb24d2..b599b6ca21 100644 --- a/contrib/pgstattuple/pgstattuple.c +++ b/contrib/pgstattuple/pgstattuple.c @@ -89,7 +89,7 @@ static Datum build_pgstattuple_type(pgstattuple_type *stat, FunctionCallInfo fcinfo) { #define NCOLUMNS 9 -#define NCHARS 32 +#define NCHARS 314 HeapTuple tuple; char *values[NCOLUMNS]; diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c index 900fa74e85..f0dfef5a86 100644 --- a/src/backend/commands/explain.c +++ b/src/backend/commands/explain.c @@ -3337,10 +3337,11 @@ void ExplainPropertyFloat(const char *qlabel, double value, int ndigits, ExplainState *es) { - char buf[256]; + char *buf; - snprintf(buf, sizeof(buf), "%.*f", ndigits, value); + buf = psprintf("%.*f", ndigits, value); ExplainProperty(qlabel, buf, true, es); + pfree(buf); } /* diff --git a/src/backend/libpq/be-secure-openssl.c b/src/backend/libpq/be-secure-openssl.c index e1ddfb3c16..567cf7d455 100644 --- a/src/backend/libpq/be-secure-openssl.c +++ b/src/backend/libpq/be-secure-openssl.c @@ -1013,7 +1013,7 @@ static const char * SSLerrmessage(unsigned long ecode) { const char *errreason; - static char errbuf[32]; + static char errbuf[36]; if (ecode == 0) return _("no SSL error reported"); diff --git a/src/backend/utils/adt/dbsize.c b/src/backend/utils/adt/dbsize.c index 834a10485f..07e5e78caa 100644 --- a/src/backend/utils/adt/dbsize.c +++ b/src/backend/utils/adt/dbsize.c @@ -86,7 +86,7 @@ calculate_database_size(Oid dbOid) DIR *dirdesc; struct dirent *direntry; char dirpath[MAXPGPATH]; - char pathname[MAXPGPATH + 12 + sizeof(TABLESPACE_VERSION_DIRECTORY)]; + char pathname[MAXPGPATH + 21 + sizeof(TABLESPACE_VERSION_DIRECTORY)]; AclResult aclresult; /* diff --git a/src/backend/utils/adt/float.c b/src/backend/utils/adt/float.c index aadb92de66..6522c0816e 100644 --- a/src/backend/utils/adt/float.c +++ b/src/backend/utils/adt/float.c @@ -44,10 +44,6 @@ static const uint32 nan[2] = {0xffffffff, 0x7fffffff}; #define NAN (*(const double *) nan) #endif -/* not sure what the following should be, but better to make it over-sufficient */ -#define MAXFLOATWIDTH 64 -#define MAXDOUBLEWIDTH 128 - /* * check to see if a float4/8 val has underflowed or overflowed */ @@ -360,18 +356,18 @@ Datum float4out(PG_FUNCTION_ARGS) { float4 num = PG_GETARG_FLOAT4(0); - char *ascii = (char *) palloc(MAXFLOATWIDTH + 1); + char *ascii; if (isnan(num)) - PG_RETURN_CSTRING(strcpy(ascii, "NaN")); + PG_RETURN_CSTRING(pstrdup("NaN")); switch (is_infinite(num)) { case 1: - strcpy(ascii, "Infinity"); + ascii = pstrdup("Infinity"); break; case -1: - strcpy(ascii, "-Infinity"); + ascii = pstrdup("-Infinity"); break; default: { @@ -380,7 +376,7 @@ float4out(PG_FUNCTION_ARGS) if (ndig < 1) ndig = 1; - snprintf(ascii, MAXFLOATWIDTH + 1, "%.*g", ndig, num); + ascii = psprintf("%.*g", ndig, num); } } @@ -596,18 +592,18 @@ float8out(PG_FUNCTION_ARGS) char * float8out_internal(double num) { - char *ascii = (char *) palloc(MAXDOUBLEWIDTH + 1); + char *ascii; if (isnan(num)) - return strcpy(ascii, "NaN"); + return pstrdup("NaN"); switch (is_infinite(num)) { case 1: - strcpy(ascii, "Infinity"); + ascii = pstrdup("Infinity"); break; case -1: - strcpy(ascii, "-Infinity"); + ascii = pstrdup("-Infinity"); break; default: { @@ -616,7 +612,7 @@ float8out_internal(double num) if (ndig < 1) ndig = 1; - snprintf(ascii, MAXDOUBLEWIDTH + 1, "%.*g", ndig, num); + ascii = psprintf("%.*g", ndig, num); } } diff --git a/src/backend/utils/adt/formatting.c b/src/backend/utils/adt/formatting.c index b8bd4caa3e..1a1088711c 100644 --- a/src/backend/utils/adt/formatting.c +++ b/src/backend/utils/adt/formatting.c @@ -117,13 +117,6 @@ #define DCH_MAX_ITEM_SIZ 12 /* max localized day name */ #define NUM_MAX_ITEM_SIZ 8 /* roman number (RN has 15 chars) */ -/* ---------- - * More is in float.c - * ---------- - */ -#define MAXFLOATWIDTH 60 -#define MAXDOUBLEWIDTH 500 - /* ---------- * Format parser structs @@ -3911,9 +3904,7 @@ do_to_timestamp(text *date_txt, text *fmt, tmfc.tzm < 0 || tmfc.tzm >= MINS_PER_HOUR) DateTimeParseError(DTERR_TZDISP_OVERFLOW, date_str, "timestamp"); - tz = palloc(7); - - snprintf(tz, 7, "%c%02d:%02d", + tz = psprintf("%c%02d:%02d", tmfc.tzsign > 0 ? '+' : '-', tmfc.tzh, tmfc.tzm); tm->tm_zone = tz; @@ -4135,7 +4126,7 @@ int_to_roman(int number) num = 0; char *p = NULL, *result, - numstr[5]; + numstr[12]; result = (char *) palloc(16); *result = '\0'; @@ -5441,8 +5432,7 @@ int4_to_char(PG_FUNCTION_ARGS) /* we can do it easily because float8 won't lose any precision */ float8 val = (float8) value; - orgnum = (char *) palloc(MAXDOUBLEWIDTH + 1); - snprintf(orgnum, MAXDOUBLEWIDTH + 1, "%+.*e", Num.post, val); + orgnum = (char *) psprintf("%+.*e", Num.post, val); /* * Swap a leading positive sign for a space. @@ -5641,7 +5631,6 @@ float4_to_char(PG_FUNCTION_ARGS) numstr = orgnum = int_to_roman((int) rint(value)); else if (IS_EEEE(&Num)) { - numstr = orgnum = (char *) palloc(MAXDOUBLEWIDTH + 1); if (isnan(value) || is_infinite(value)) { /* @@ -5655,7 +5644,7 @@ float4_to_char(PG_FUNCTION_ARGS) } else { - snprintf(orgnum, MAXDOUBLEWIDTH + 1, "%+.*e", Num.post, value); + numstr = orgnum = psprintf("%+.*e", Num.post, value); /* * Swap a leading positive sign for a space. @@ -5679,8 +5668,7 @@ float4_to_char(PG_FUNCTION_ARGS) Num.pre += Num.multi; } - orgnum = (char *) palloc(MAXFLOATWIDTH + 1); - snprintf(orgnum, MAXFLOATWIDTH + 1, "%.0f", fabs(val)); + orgnum = (char *) psprintf("%.0f", fabs(val)); numstr_pre_len = strlen(orgnum); /* adjust post digits to fit max float digits */ @@ -5688,7 +5676,7 @@ float4_to_char(PG_FUNCTION_ARGS) Num.post = 0; else if (numstr_pre_len + Num.post > FLT_DIG) Num.post = FLT_DIG - numstr_pre_len; - snprintf(orgnum, MAXFLOATWIDTH + 1, "%.*f", Num.post, val); + orgnum = psprintf("%.*f", Num.post, val); if (*orgnum == '-') { /* < 0 */ @@ -5747,7 +5735,6 @@ float8_to_char(PG_FUNCTION_ARGS) numstr = orgnum = int_to_roman((int) rint(value)); else if (IS_EEEE(&Num)) { - numstr = orgnum = (char *) palloc(MAXDOUBLEWIDTH + 1); if (isnan(value) || is_infinite(value)) { /* @@ -5761,7 +5748,7 @@ float8_to_char(PG_FUNCTION_ARGS) } else { - snprintf(orgnum, MAXDOUBLEWIDTH + 1, "%+.*e", Num.post, value); + numstr = orgnum = (char *) psprintf("%+.*e", Num.post, value); /* * Swap a leading positive sign for a space. @@ -5784,15 +5771,15 @@ float8_to_char(PG_FUNCTION_ARGS) val = value * multi; Num.pre += Num.multi; } - orgnum = (char *) palloc(MAXDOUBLEWIDTH + 1); - numstr_pre_len = snprintf(orgnum, MAXDOUBLEWIDTH + 1, "%.0f", fabs(val)); + orgnum = psprintf("%.0f", fabs(val)); + numstr_pre_len = strlen(orgnum); /* adjust post digits to fit max double digits */ if (numstr_pre_len >= DBL_DIG) Num.post = 0; else if (numstr_pre_len + Num.post > DBL_DIG) Num.post = DBL_DIG - numstr_pre_len; - snprintf(orgnum, MAXDOUBLEWIDTH + 1, "%.*f", Num.post, val); + orgnum = psprintf("%.*f", Num.post, val); if (*orgnum == '-') { /* < 0 */ diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c index fc3e10c750..4116c2d66a 100644 --- a/src/backend/utils/misc/guc.c +++ b/src/backend/utils/misc/guc.c @@ -10528,7 +10528,7 @@ check_cluster_name(char **newval, void **extra, GucSource source) static const char * show_unix_socket_permissions(void) { - static char buf[8]; + static char buf[12]; snprintf(buf, sizeof(buf), "%04o", Unix_socket_permissions); return buf; @@ -10537,7 +10537,7 @@ show_unix_socket_permissions(void) static const char * show_log_file_mode(void) { - static char buf[8]; + static char buf[12]; snprintf(buf, sizeof(buf), "%04o", Log_file_mode); return buf; diff --git a/src/bin/initdb/initdb.c b/src/bin/initdb/initdb.c index 65eba7d42f..feee359169 100644 --- a/src/bin/initdb/initdb.c +++ b/src/bin/initdb/initdb.c @@ -1009,12 +1009,12 @@ static char * pretty_wal_size(int segment_count) { int sz = wal_segment_size_mb * segment_count; - char *result = pg_malloc(11); + char *result = pg_malloc(14); if ((sz % 1024) == 0) - snprintf(result, 11, "%dGB", sz / 1024); + snprintf(result, 14, "%dGB", sz / 1024); else - snprintf(result, 11, "%dMB", sz); + snprintf(result, 14, "%dMB", sz); return result; } diff --git a/src/bin/pg_dump/pg_backup_archiver.c b/src/bin/pg_dump/pg_backup_archiver.c index fc233a608f..83c976eaf7 100644 --- a/src/bin/pg_dump/pg_backup_archiver.c +++ b/src/bin/pg_dump/pg_backup_archiver.c @@ -1532,7 +1532,7 @@ SetOutput(ArchiveHandle *AH, const char *filename, int compression) #ifdef HAVE_LIBZ if (compression != 0) { - char fmode[10]; + char fmode[14]; /* Don't use PG_BINARY_x since this is zlib */ sprintf(fmode, "wb%d", compression); diff --git a/src/bin/pg_dump/pg_backup_tar.c b/src/bin/pg_dump/pg_backup_tar.c index ef9f7145b1..007be1298f 100644 --- a/src/bin/pg_dump/pg_backup_tar.c +++ b/src/bin/pg_dump/pg_backup_tar.c @@ -335,7 +335,7 @@ tarOpen(ArchiveHandle *AH, const char *filename, char mode) TAR_MEMBER *tm; #ifdef HAVE_LIBZ - char fmode[10]; + char fmode[14]; #endif if (mode == 'r') diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c index 5c07dd9ca5..29d69de4d1 100644 --- a/src/bin/pgbench/pgbench.c +++ b/src/bin/pgbench/pgbench.c @@ -3591,7 +3591,7 @@ parseQuery(Command *cmd) p = sql; while ((p = strchr(p, ':')) != NULL) { - char var[12]; + char var[13]; char *name; int eaten; @@ -5432,7 +5432,7 @@ threadRun(void *arg) sqlat, lag, stdev; - char tbuf[64]; + char tbuf[315]; /* * Add up the statistics of all threads. diff --git a/src/include/postmaster/bgworker.h b/src/include/postmaster/bgworker.h index 0c04529f47..a8753df8d1 100644 --- a/src/include/postmaster/bgworker.h +++ b/src/include/postmaster/bgworker.h @@ -82,7 +82,7 @@ typedef enum #define BGW_DEFAULT_RESTART_INTERVAL 60 #define BGW_NEVER_RESTART -1 -#define BGW_MAXLEN 64 +#define BGW_MAXLEN 96 #define BGW_EXTRALEN 128 typedef struct BackgroundWorker diff --git a/src/interfaces/libpq/fe-secure-openssl.c b/src/interfaces/libpq/fe-secure-openssl.c index cade4e157c..127122563c 100644 --- a/src/interfaces/libpq/fe-secure-openssl.c +++ b/src/interfaces/libpq/fe-secure-openssl.c @@ -1436,7 +1436,7 @@ PQsslAttribute(PGconn *conn, const char *attribute_name) if (strcmp(attribute_name, "key_bits") == 0) { - static char sslbits_str[10]; + static char sslbits_str[12]; int sslbits; SSL_get_cipher_bits(conn->ssl, &sslbits); diff --git a/src/pl/tcl/pltcl.c b/src/pl/tcl/pltcl.c index 11411f35ab..865071bc3b 100644 --- a/src/pl/tcl/pltcl.c +++ b/src/pl/tcl/pltcl.c @@ -1456,7 +1456,7 @@ compile_pltcl_function(Oid fn_oid, Oid tgreloid, Datum prosrcdatum; bool isnull; char *proc_source; - char buf[32]; + char buf[48]; Tcl_Interp *interp; int i; int tcl_rc; -- 2.40.0