pg.ddx.io  pgsql-hackers@postgresql.org mailing list archive  
help / color / mirror / Atom feed
GCC 7 warnings
13+ messages / 5 participants
[nested] [flat]

* GCC 7 warnings
@ 2017-04-10 15:03 Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
  2017-04-10 15:35 ` Re: GCC 7 warnings Aleksander Alekseev <a.alekseev@postgrespro.ru>
  2017-04-10 16:10 ` Re: GCC 7 warnings Andres Freund <andres@anarazel.de>
  2017-04-10 16:25 ` Re: GCC 7 warnings Tom Lane <tgl@sss.pgh.pa.us>
  2017-05-04 01:59 ` Re: GCC 7 warnings Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
  0 siblings, 4 replies; 13+ messages in thread

From: Peter Eisentraut @ 2017-04-10 15:03 UTC (permalink / raw)
  To: pgsql-hackers

The release of GCC 7 is approaching [0], and the number of warnings in
PostgreSQL has gone up since we last looked [1].  Output attached.  (My
version is 7.0.1 20170408.)

Most of the issues have to do with concatenating two or more strings of
potential size MAXPGPATH into another buffer of size MAXPGPATH, which
could lead to truncation.

Possible fixes:

a) Ignore, hoping GCC will change before final release. (unlikely at
this point)

b) Add compiler option to disable this particular warning, worry about
it later.  (Might be an option for backpatching.)

c) Expand the target buffer sizes until the warning goes away.  (Sample
patch attached.)

d) Replace most of the problematic code with psprintf() and dynamically
sized buffers.

Comments?


[0]: https://gcc.gnu.org/ml/gcc/2017-03/msg00066.html
[1]:
https://www.postgresql.org/message-id/CAFj8pRA=xV0_-aDF5UtGVY8HGvg+ovA_js_P8z4jLHxvX0Wa=A@mail.gmail...

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Attachments:

gcc-7-warnings.txt (text/plain, 25.1K) download In file included from /usr/include/stdio.h:498:0, from ../../src/include/c.h:81, from ../../src/include/postgres_fe.h:25, from file_utils.c:15: file_utils.c: In function 'walkdir.constprop': file_utils.c:177:32: warning: '__builtin_snprintf' output may be truncated before the last format character [-Wformat-truncation=] snprintf(subpath, MAXPGPATH, "%s/%s", path, de->d_name); ^ file_utils.c:177:3: note: '__builtin_snprintf' output 2 or more bytes (assuming 1025) into a destination of size 1024 snprintf(subpath, MAXPGPATH, "%s/%s", path, de->d_name); ^ In file included from /usr/include/stdio.h:498:0, from ../../src/include/c.h:81, from ../../src/include/postgres.h:47, from pgtz.c:13: pgtz.c: In function 'pg_tzenumerate_next': pgtz.c:432:33: warning: '__builtin_snprintf' output may be truncated before the last format character [-Wformat-truncation=] snprintf(fullname, MAXPGPATH, "%s/%s", ^ pgtz.c:432:3: note: '__builtin_snprintf' output 2 or more bytes (assuming 1025) into a destination of size 1024 snprintf(fullname, MAXPGPATH, "%s/%s", ^ In file included from /usr/include/stdio.h:498:0, from ../../../../src/include/c.h:81, from ../../../../src/include/postgres.h:47, from rewriteheap.c:103: rewriteheap.c: In function 'CheckPointLogicalRewriteHeap': rewriteheap.c:1237:29: warning: '%s' directive output may be truncated writing up to 1023 bytes into a region of size 1004 [-Wformat-truncation=] snprintf(path, MAXPGPATH, "pg_logical/mappings/%s", mapping_de->d_name); ^ rewriteheap.c:1237:3: note: '__builtin_snprintf' output between 21 and 1044 bytes into a destination of size 1024 snprintf(path, MAXPGPATH, "pg_logical/mappings/%s", mapping_de->d_name); ^ In file included from /usr/include/stdio.h:498:0, from ../../../../src/include/c.h:81, from ../../../../src/include/postgres.h:47, from xlog.c:15: xlog.c: In function 'do_pg_start_backup': xlog.c:10403:41: warning: '%s' directive output may be truncated writing up to 1023 bytes into a region of size 1014 [-Wformat-truncation=] snprintf(fullpath, sizeof(fullpath), "pg_tblspc/%s", de->d_name); ^ xlog.c:10403:4: note: '__builtin_snprintf' output between 11 and 1034 bytes into a destination of size 1024 snprintf(fullpath, sizeof(fullpath), "pg_tblspc/%s", de->d_name); ^ xlog.c: In function 'do_pg_stop_backup': ../../../../src/include/access/xlog_internal.h:131:20: warning: '%s' directive output may be truncated writing up to 1023 bytes into a region of size 1017 [-Wformat-truncation=] #define XLOGDIR "pg_wal" ^ xlog.c:4123:31: note: in expansion of macro 'XLOGDIR' snprintf(path, MAXPGPATH, XLOGDIR "/%s", xlde->d_name); ^~~~~~~ In file included from xlog.c:35:0: ../../../../src/include/access/xlog_internal.h:203:38: note: format string is defined here snprintf(path, MAXPGPATH, XLOGDIR "/%08X%08X%08X.%08X.backup", tli, \ ^~ In file included from /usr/include/stdio.h:498:0, from ../../../../src/include/c.h:81, from ../../../../src/include/postgres.h:47, from xlog.c:15: xlog.c:4123:5: note: '__builtin_snprintf' output between 8 and 1031 bytes into a destination of size 1024 snprintf(path, MAXPGPATH, XLOGDIR "/%s", xlde->d_name); ^ In file included from /usr/include/stdio.h:498:0, from ../../../src/include/c.h:81, from ../../../src/include/postgres.h:47, from pgstat.c:19: pgstat.c: In function 'pgstat_reset_remove_files': pgstat.c:639:30: warning: '__builtin_snprintf' output may be truncated before the last format character [-Wformat-truncation=] snprintf(fname, MAXPGPATH, "%s/%s", directory, ^ pgstat.c:639:3: note: '__builtin_snprintf' output 2 or more bytes (assuming 1025) into a destination of size 1024 snprintf(fname, MAXPGPATH, "%s/%s", directory, ^ In file included from /usr/include/stdio.h:498:0, from ../../../../src/include/c.h:81, from ../../../../src/include/postgres.h:47, from reorderbuffer.c:48: reorderbuffer.c: In function 'StartupReorderBuffer': reorderbuffer.c:2630:17: warning: '%s' directive writing up to 1023 bytes into a region of size 1012 [-Wformat-overflow=] sprintf(path, "pg_replslot/%s", logical_de->d_name); ^ reorderbuffer.c:2630:3: note: '__builtin___sprintf_chk' output between 13 and 1036 bytes into a destination of size 1024 sprintf(path, "pg_replslot/%s", logical_de->d_name); ^ reorderbuffer.c:2646:19: warning: '%s' directive writing up to 1023 bytes into a region of size 1012 [-Wformat-overflow=] sprintf(path, "pg_replslot/%s/%s", logical_de->d_name, ^ reorderbuffer.c:2646:5: note: '__builtin___sprintf_chk' output between 14 and 2060 bytes into a destination of size 1024 sprintf(path, "pg_replslot/%s/%s", logical_de->d_name, ^ In file included from /usr/include/stdio.h:498:0, from ../../../../src/include/c.h:81, from ../../../../src/include/postgres.h:47, from snapbuild.c:107: snapbuild.c: In function 'CheckPointSnapBuild': snapbuild.c:1898:29: warning: '%s' directive output may be truncated writing up to 1023 bytes into a region of size 1003 [-Wformat-truncation=] snprintf(path, MAXPGPATH, "pg_logical/snapshots/%s", snap_de->d_name); ^ snapbuild.c:1898:3: note: '__builtin_snprintf' output between 22 and 1045 bytes into a destination of size 1024 snprintf(path, MAXPGPATH, "pg_logical/snapshots/%s", snap_de->d_name); ^ In file included from /usr/include/stdio.h:498:0, from ../../../src/include/c.h:81, from ../../../src/include/postgres.h:47, from basebackup.c:13: basebackup.c: In function 'sendDir': basebackup.c:1014:32: warning: '__builtin_snprintf' output may be truncated before the last format character [-Wformat-truncation=] snprintf(pathbuf, MAXPGPATH, "%s/%s", path, de->d_name); ^ basebackup.c:1014:3: note: '__builtin_snprintf' output 2 or more bytes (assuming 1025) into a destination of size 1024 snprintf(pathbuf, MAXPGPATH, "%s/%s", path, de->d_name); ^ In file included from /usr/include/stdio.h:498:0, from ../../../src/include/c.h:81, from ../../../src/include/postgres.h:47, from slot.c:37: slot.c: In function 'StartupReplicationSlots': slot.c:1032:29: warning: '%s' directive output may be truncated writing up to 1023 bytes into a region of size 1012 [-Wformat-truncation=] snprintf(path, MAXPGPATH, "pg_replslot/%s", replication_de->d_name); ^ slot.c:1032:3: note: '__builtin_snprintf' output between 13 and 1036 bytes into a destination of size 1024 snprintf(path, MAXPGPATH, "pg_replslot/%s", replication_de->d_name); ^ slot.c:1271:16: warning: '%s' directive writing up to 1023 bytes into a region of size 1012 [-Wformat-overflow=] sprintf(path, "pg_replslot/%s/state.tmp", name); ^ slot.c:1271:2: note: '__builtin___sprintf_chk' output between 23 and 1046 bytes into a destination of size 1024 sprintf(path, "pg_replslot/%s/state.tmp", name); ^ slot.c:1277:16: warning: '%s' directive writing up to 1023 bytes into a region of size 1012 [-Wformat-overflow=] sprintf(path, "pg_replslot/%s/state", name); ^ slot.c:1277:2: note: '__builtin___sprintf_chk' output between 19 and 1042 bytes into a destination of size 1024 sprintf(path, "pg_replslot/%s/state", name); ^ slot.c:1388:17: warning: '%s' directive writing up to 1023 bytes into a region of size 1012 [-Wformat-overflow=] sprintf(path, "pg_replslot/%s", name); ^ slot.c:1388:3: note: '__builtin___sprintf_chk' output between 13 and 1036 bytes into a destination of size 1024 sprintf(path, "pg_replslot/%s", name); ^ In file included from /usr/include/stdio.h:498:0, from ../../../../src/include/c.h:81, from ../../../../src/include/postgres.h:47, from fd.c:59: fd.c: In function 'RemovePgTempFilesInDir': fd.c:2775:38: warning: '__builtin_snprintf' output may be truncated before the last format character [-Wformat-truncation=] snprintf(rm_path, sizeof(rm_path), "%s/%s", ^ fd.c:2775:3: note: '__builtin_snprintf' output 2 or more bytes (assuming 1025) into a destination of size 1024 snprintf(rm_path, sizeof(rm_path), "%s/%s", ^ fd.c: In function 'RemovePgTempRelationFiles': fd.c:2824:48: warning: '__builtin_snprintf' output may be truncated before the last format character [-Wformat-truncation=] snprintf(dbspace_path, sizeof(dbspace_path), "%s/%s", ^ fd.c:2824:3: note: '__builtin_snprintf' output 2 or more bytes (assuming 1025) into a destination of size 1024 snprintf(dbspace_path, sizeof(dbspace_path), "%s/%s", ^ fd.c:2855:38: warning: '__builtin_snprintf' output may be truncated before the last format character [-Wformat-truncation=] snprintf(rm_path, sizeof(rm_path), "%s/%s", ^ fd.c:2855:3: note: '__builtin_snprintf' output 2 or more bytes (assuming 1025) into a destination of size 1024 snprintf(rm_path, sizeof(rm_path), "%s/%s", ^ fd.c: In function 'walkdir': fd.c:3035:32: warning: '__builtin_snprintf' output may be truncated before the last format character [-Wformat-truncation=] snprintf(subpath, MAXPGPATH, "%s/%s", path, de->d_name); ^ fd.c:3035:3: note: '__builtin_snprintf' output 2 or more bytes (assuming 1025) into a destination of size 1024 snprintf(subpath, MAXPGPATH, "%s/%s", path, de->d_name); ^ fd.c: In function 'RemovePgTempFiles': fd.c:2730:42: warning: '%s' directive output may be truncated writing up to 1023 bytes into a region of size 1014 [-Wformat-truncation=] snprintf(temp_path, sizeof(temp_path), "pg_tblspc/%s/%s/%s", ^ fd.c:2730:3: note: '__builtin_snprintf' output between 37 and 1060 bytes into a destination of size 1024 snprintf(temp_path, sizeof(temp_path), "pg_tblspc/%s/%s/%s", ^ fd.c:2734:42: warning: '%s' directive output may be truncated writing up to 1023 bytes into a region of size 1014 [-Wformat-truncation=] snprintf(temp_path, sizeof(temp_path), "pg_tblspc/%s/%s", ^ fd.c:2734:3: note: '__builtin_snprintf' output between 27 and 1050 bytes into a destination of size 1024 snprintf(temp_path, sizeof(temp_path), "pg_tblspc/%s/%s", ^ In file included from /usr/include/stdio.h:498:0, from ../../../../src/include/c.h:81, from ../../../../src/include/postgres.h:47, from copydir.c:19: copydir.c: In function 'copydir': copydir.c:66:33: warning: '__builtin_snprintf' output may be truncated before the last format character [-Wformat-truncation=] snprintf(fromfile, MAXPGPATH, "%s/%s", fromdir, xlde->d_name); ^ copydir.c:66:3: note: '__builtin_snprintf' output 2 or more bytes (assuming 1025) into a destination of size 1024 snprintf(fromfile, MAXPGPATH, "%s/%s", fromdir, xlde->d_name); ^ copydir.c:67:31: warning: '__builtin_snprintf' output may be truncated before the last format character [-Wformat-truncation=] snprintf(tofile, MAXPGPATH, "%s/%s", todir, xlde->d_name); ^ copydir.c:67:3: note: '__builtin_snprintf' output 2 or more bytes (assuming 1025) into a destination of size 1024 snprintf(tofile, MAXPGPATH, "%s/%s", todir, xlde->d_name); ^ copydir.c:106:31: warning: '__builtin_snprintf' output may be truncated before the last format character [-Wformat-truncation=] snprintf(tofile, MAXPGPATH, "%s/%s", todir, xlde->d_name); ^ copydir.c:106:3: note: '__builtin_snprintf' output 2 or more bytes (assuming 1025) into a destination of size 1024 snprintf(tofile, MAXPGPATH, "%s/%s", todir, xlde->d_name); ^ In file included from /usr/include/stdio.h:498:0, from ../../../../src/include/c.h:81, from ../../../../src/include/postgres.h:47, from reinit.c:15: reinit.c: In function 'ResetUnloggedRelationsInTablespaceDir': reinit.c:134:48: warning: '__builtin_snprintf' output may be truncated before the last format character [-Wformat-truncation=] snprintf(dbspace_path, sizeof(dbspace_path), "%s/%s", ^ reinit.c:134:3: note: '__builtin_snprintf' output 2 or more bytes (assuming 1025) into a destination of size 1024 snprintf(dbspace_path, sizeof(dbspace_path), "%s/%s", ^ reinit.c:264:40: warning: '__builtin_snprintf' output may be truncated before the last format character [-Wformat-truncation=] snprintf(rm_path, sizeof(rm_path), "%s/%s", ^ reinit.c:264:5: note: '__builtin_snprintf' output 2 or more bytes (assuming 1025) into a destination of size 1024 snprintf(rm_path, sizeof(rm_path), "%s/%s", ^ reinit.c:324:39: warning: '__builtin_snprintf' output may be truncated before the last format character [-Wformat-truncation=] snprintf(srcpath, sizeof(srcpath), "%s/%s", ^ reinit.c:324:4: note: '__builtin_snprintf' output 2 or more bytes (assuming 1025) into a destination of size 1024 snprintf(srcpath, sizeof(srcpath), "%s/%s", ^ reinit.c: In function 'ResetUnloggedRelations': reinit.c:87:42: warning: '%s' directive output may be truncated writing up to 1023 bytes into a region of size 1014 [-Wformat-truncation=] snprintf(temp_path, sizeof(temp_path), "pg_tblspc/%s/%s", ^ reinit.c:87:3: note: '__builtin_snprintf' output between 27 and 1050 bytes into a destination of size 1024 snprintf(temp_path, sizeof(temp_path), "pg_tblspc/%s/%s", ^ In file included from /usr/include/stdio.h:498:0, from ../../../../src/include/c.h:81, from ../../../../src/include/postgres.h:47, from dsm.c:27: dsm.c: In function 'dsm_cleanup_for_mmap': ../../../../src/include/storage/dsm_impl.h:51:29: warning: '%s' directive output may be truncated writing up to 1023 bytes into a region of size 1012 [-Wformat-truncation=] #define PG_DYNSHMEM_DIR "pg_dynshmem" ^ dsm.c:312:29: note: in expansion of macro 'PG_DYNSHMEM_DIR' snprintf(buf, MAXPGPATH, PG_DYNSHMEM_DIR "/%s", dent->d_name); ^~~~~~~~~~~~~~~ dsm.c:312:47: note: format string is defined here snprintf(buf, MAXPGPATH, PG_DYNSHMEM_DIR "/%s", dent->d_name); ^~ In file included from /usr/include/stdio.h:498:0, from ../../../../src/include/c.h:81, from ../../../../src/include/postgres.h:47, from dsm.c:27: dsm.c:312:4: note: '__builtin_snprintf' output between 13 and 1036 bytes into a destination of size 1024 snprintf(buf, MAXPGPATH, PG_DYNSHMEM_DIR "/%s", dent->d_name); ^ In file included from /usr/include/stdio.h:498:0, from ../../../../src/include/c.h:81, from ../../../../src/include/postgres.h:47, from dbsize.c:12: dbsize.c: In function 'db_dir_size': dbsize.c:61:33: warning: '__builtin_snprintf' output may be truncated before the last format character [-Wformat-truncation=] snprintf(filename, MAXPGPATH, "%s/%s", path, direntry->d_name); ^ dbsize.c:61:3: note: '__builtin_snprintf' output 2 or more bytes (assuming 1025) into a destination of size 1024 snprintf(filename, MAXPGPATH, "%s/%s", path, direntry->d_name); ^ dbsize.c: In function 'calculate_database_size': dbsize.c:127:33: warning: '%s' directive output may be truncated writing up to 1023 bytes into a region of size 1014 [-Wformat-truncation=] snprintf(pathname, MAXPGPATH, "pg_tblspc/%s/%s/%u", ^ dbsize.c:127:3: note: '__builtin_snprintf' output between 29 and 1061 bytes into a destination of size 1024 snprintf(pathname, MAXPGPATH, "pg_tblspc/%s/%s/%u", ^ dbsize.c: In function 'calculate_tablespace_size': dbsize.c:218:33: warning: '__builtin_snprintf' output may be truncated before the last format character [-Wformat-truncation=] snprintf(pathname, MAXPGPATH, "%s/%s", tblspcPath, direntry->d_name); ^ dbsize.c:218:3: note: '__builtin_snprintf' output 2 or more bytes (assuming 1025) into a destination of size 1024 snprintf(pathname, MAXPGPATH, "%s/%s", tblspcPath, direntry->d_name); ^ In file included from /usr/include/stdio.h:498:0, from ../../../../src/include/c.h:81, from ../../../../src/include/postgres.h:47, from genfile.c:16: genfile.c: In function 'pg_ls_dir_files': genfile.c:534:29: warning: '__builtin_snprintf' output may be truncated before the last format character [-Wformat-truncation=] snprintf(path, MAXPGPATH, "%s/%s", fctx->location, de->d_name); ^ genfile.c:534:3: note: '__builtin_snprintf' output 2 or more bytes (assuming 1025) into a destination of size 1024 snprintf(path, MAXPGPATH, "%s/%s", fctx->location, de->d_name); ^ In file included from /usr/include/stdio.h:498:0, from ../../../../src/include/c.h:81, from ../../../../src/include/postgres.h:47, from relcache.c:27: relcache.c: In function 'RelationCacheInitFileRemoveInDir': relcache.c:6135:49: warning: '/' directive output may be truncated writing 1 byte into a region of size between 0 and 1023 [-Wformat-truncation=] snprintf(initfilename, sizeof(initfilename), "%s/%s/%s", ^ relcache.c:6135:4: note: '__builtin_snprintf' output 19 or more bytes (assuming 1042) into a destination of size 1024 snprintf(initfilename, sizeof(initfilename), "%s/%s/%s", ^ relcache.c: In function 'RelationCacheInitFileRemove': relcache.c:6104:33: warning: '%s' directive output may be truncated writing up to 1023 bytes into a region of size 1014 [-Wformat-truncation=] snprintf(path, sizeof(path), "%s/%s/%s", ^ relcache.c:6104:4: note: '__builtin_snprintf' output between 27 and 1050 bytes into a destination of size 1024 snprintf(path, sizeof(path), "%s/%s/%s", ^ In file included from /usr/include/stdio.h:498:0, from ../../../../src/include/c.h:81, from ../../../../src/include/postgres.h:47, from elog.c:55: elog.c: In function 'setup_formatted_log_time': elog.c:2252:17: warning: '%03d' directive writing between 3 and 8 bytes into a region of size 7 [-Wformat-overflow=] sprintf(msbuf, ".%03d", (int) (saved_timeval.tv_usec / 1000)); ^ elog.c:2252:17: note: directive argument in the range [-2147483, 2147483] elog.c:2252:2: note: '__builtin___sprintf_chk' output between 5 and 10 bytes into a destination of size 8 sprintf(msbuf, ".%03d", (int) (saved_timeval.tv_usec / 1000)); ^ In file included from /usr/include/stdio.h:498:0, from ../../../../src/include/c.h:81, from ../../../../src/include/postgres.h:47, from snapmgr.c:46: snapmgr.c: In function 'DeleteAllExportedSnapshotFiles': snapmgr.c:213:29: warning: '%s' directive output may be truncated writing up to 1023 bytes into a region of size 1011 [-Wformat-truncation=] #define SNAPSHOT_EXPORT_DIR "pg_snapshots" ^ snapmgr.c:1602:28: note: in expansion of macro 'SNAPSHOT_EXPORT_DIR' snprintf(buf, MAXPGPATH, SNAPSHOT_EXPORT_DIR "/%s", s_de->d_name); ^~~~~~~~~~~~~~~~~~~ snapmgr.c:1602:50: note: format string is defined here snprintf(buf, MAXPGPATH, SNAPSHOT_EXPORT_DIR "/%s", s_de->d_name); ^~ In file included from /usr/include/stdio.h:498:0, from ../../../../src/include/c.h:81, from ../../../../src/include/postgres.h:47, from snapmgr.c:46: snapmgr.c:1602:3: note: '__builtin_snprintf' output between 14 and 1037 bytes into a destination of size 1024 snprintf(buf, MAXPGPATH, SNAPSHOT_EXPORT_DIR "/%s", s_de->d_name); ^ In file included from /usr/include/stdio.h:498:0, from ../../../src/include/c.h:81, from ../../../src/include/postgres_fe.h:25, from pg_archivecleanup.c:13: pg_archivecleanup.c: In function 'CleanupPriorWALFiles': pg_archivecleanup.c:136:38: warning: '__builtin_snprintf' output may be truncated before the last format character [-Wformat-truncation=] snprintf(WALFilePath, MAXPGPATH, "%s/%s", ^ pg_archivecleanup.c:136:5: note: '__builtin_snprintf' output 2 or more bytes (assuming 1025) into a destination of size 1024 snprintf(WALFilePath, MAXPGPATH, "%s/%s", ^ In file included from /usr/include/stdio.h:498:0, from ../../../src/include/c.h:81, from ../../../src/include/postgres_fe.h:25, from pg_receivewal.c:15: pg_receivewal.c: In function 'FindStreamingStart': pg_receivewal.c:272:41: warning: '__builtin_snprintf' output may be truncated before the last format character [-Wformat-truncation=] snprintf(fullpath, sizeof(fullpath), "%s/%s", basedir, dirent->d_name); ^ pg_receivewal.c:272:4: note: '__builtin_snprintf' output 2 or more bytes (assuming 1025) into a destination of size 1024 snprintf(fullpath, sizeof(fullpath), "%s/%s", basedir, dirent->d_name); ^ pg_receivewal.c:249:41: warning: '__builtin_snprintf' output may be truncated before the last format character [-Wformat-truncation=] snprintf(fullpath, sizeof(fullpath), "%s/%s", basedir, dirent->d_name); ^ pg_receivewal.c:249:4: note: '__builtin_snprintf' output 2 or more bytes (assuming 1025) into a destination of size 1024 snprintf(fullpath, sizeof(fullpath), "%s/%s", basedir, dirent->d_name); ^ In file included from /usr/include/stdio.h:498:0, from ../../../src/include/c.h:81, from ../../../src/include/postgres.h:47, from pg_resetwal.c:38: pg_resetwal.c: In function 'KillExistingXLOG': pg_resetwal.c:976:30: warning: '%s' directive output may be truncated writing up to 1023 bytes into a region of size 1017 [-Wformat-truncation=] snprintf(path, MAXPGPATH, "%s/%s", XLOGDIR, xlde->d_name); ^ pg_resetwal.c:976:4: note: '__builtin_snprintf' output between 8 and 1031 bytes into a destination of size 1024 snprintf(path, MAXPGPATH, "%s/%s", XLOGDIR, xlde->d_name); ^ pg_resetwal.c: In function 'KillExistingArchiveStatus': pg_resetwal.c:1030:30: warning: '%s' directive output may be truncated writing up to 1023 bytes into a region of size 1002 [-Wformat-truncation=] snprintf(path, MAXPGPATH, "%s/%s", ARCHSTATDIR, xlde->d_name); ^ pg_resetwal.c:1030:4: note: '__builtin_snprintf' output between 23 and 1046 bytes into a destination of size 1024 snprintf(path, MAXPGPATH, "%s/%s", ARCHSTATDIR, xlde->d_name); ^ In file included from /usr/include/stdio.h:498:0, from ../../../src/include/c.h:81, from ../../../src/include/postgres_fe.h:25, from copy_fetch.c:10: copy_fetch.c: In function 'recurse_dir': copy_fetch.c:75:33: warning: '__builtin_snprintf' output may be truncated before the last format character [-Wformat-truncation=] snprintf(fullpath, MAXPGPATH, "%s/%s", fullparentpath, xlde->d_name); ^ copy_fetch.c:75:3: note: '__builtin_snprintf' output 2 or more bytes (assuming 1025) into a destination of size 1024 snprintf(fullpath, MAXPGPATH, "%s/%s", fullparentpath, xlde->d_name); ^ copy_fetch.c:96:30: warning: '__builtin_snprintf' output may be truncated before the last format character [-Wformat-truncation=] snprintf(path, MAXPGPATH, "%s/%s", parentpath, xlde->d_name); ^ copy_fetch.c:96:4: note: '__builtin_snprintf' output 2 or more bytes (assuming 1025) into a destination of size 1024 snprintf(path, MAXPGPATH, "%s/%s", parentpath, xlde->d_name); ^ In file included from /usr/include/stdio.h:498:0, from ../../src/include/c.h:81, from ../../src/include/postgres_fe.h:25, from pg_standby.c:24: pg_standby.c: In function 'main': pg_standby.c:264:39: warning: '__builtin_snprintf' output may be truncated before the last format character [-Wformat-truncation=] snprintf(WALFilePath, MAXPGPATH, "%s/%s", archiveLocation, xlde->d_name); ^ pg_standby.c:264:6: note: '__builtin_snprintf' output 2 or more bytes (assuming 1025) into a destination of size 1024 snprintf(WALFilePath, MAXPGPATH, "%s/%s", archiveLocation, xlde->d_name); ^
0001-Fix-various-new-warnings-from-gcc-7.patch (invalid/octet-stream, 0B) download ^ permalink raw reply [nested|flat] 13+ messages in thread

* Re: GCC 7 warnings
  2017-04-10 15:03 GCC 7 warnings Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
@ 2017-04-10 15:35 ` Aleksander Alekseev <a.alekseev@postgrespro.ru>
  3 siblings, 0 replies; 13+ messages in thread

From: Aleksander Alekseev @ 2017-04-10 15:35 UTC (permalink / raw)
  To: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>; +Cc: pgsql-hackers

Hi Peter,

> c) Expand the target buffer sizes until the warning goes away.  (Sample
> patch attached.)

I personally think it's a great patch. Unfortunately I don't have GCC
7.0 right now but at least it doesn't break anything on 6.3.1. Since
there is no rush I would suggest to add an entry to the next commitfest,
so this patch wouldn't accidentally be lost.

As a side node I believe it would be great to replace all sprintf calls
to snprintf just to be on a safe side. IIRC we did something similar to
str* procedures not a long time ago. Unless there are any objections
regarding this idea I'm willing to write such a patch.

-- 
Best regards,
Aleksander Alekseev

Attachments:

  signature.asc (application/pgp-signature, 832B) download

^ permalink  raw  reply  [nested|flat] 13+ messages in thread

* Re: GCC 7 warnings
  2017-04-10 15:03 GCC 7 warnings Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
@ 2017-04-10 16:10 ` Andres Freund <andres@anarazel.de>
  2017-04-10 16:16   ` Re: GCC 7 warnings Andres Freund <andres@anarazel.de>
  3 siblings, 1 reply; 13+ messages in thread

From: Andres Freund @ 2017-04-10 16:10 UTC (permalink / raw)
  To: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>; +Cc: pgsql-hackers

Hi,

On 2017-04-10 11:03:23 -0400, Peter Eisentraut wrote:
> The release of GCC 7 is approaching [0], and the number of warnings in
> PostgreSQL has gone up since we last looked [1].  Output attached.  (My
> version is 7.0.1 20170408.)
> 
> Most of the issues have to do with concatenating two or more strings of
> potential size MAXPGPATH into another buffer of size MAXPGPATH, which
> could lead to truncation.

Hm, interesting - I don't see this with
gcc-7 (Debian 7-20170407-1) 7.0.1 20170407 (experimental) [trunk revision 246759]
although I do recall having seen them at some point.  Not sure what's up
there.

I've to add -Wno-implicit-fallthrough to avoid noisyness, though.

Regards,

Andres


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers



^ permalink  raw  reply  [nested|flat] 13+ messages in thread

* Re: GCC 7 warnings
  2017-04-10 15:03 GCC 7 warnings Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
  2017-04-10 16:10 ` Re: GCC 7 warnings Andres Freund <andres@anarazel.de>
@ 2017-04-10 16:16   ` Andres Freund <andres@anarazel.de>
  0 siblings, 0 replies; 13+ messages in thread

From: Andres Freund @ 2017-04-10 16:16 UTC (permalink / raw)
  To: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>; +Cc: pgsql-hackers

On 2017-04-10 09:10:07 -0700, Andres Freund wrote:
> Hi,
> 
> On 2017-04-10 11:03:23 -0400, Peter Eisentraut wrote:
> > The release of GCC 7 is approaching [0], and the number of warnings in
> > PostgreSQL has gone up since we last looked [1].  Output attached.  (My
> > version is 7.0.1 20170408.)
> > 
> > Most of the issues have to do with concatenating two or more strings of
> > potential size MAXPGPATH into another buffer of size MAXPGPATH, which
> > could lead to truncation.
> 
> Hm, interesting - I don't see this with
> gcc-7 (Debian 7-20170407-1) 7.0.1 20170407 (experimental) [trunk revision 246759]
> although I do recall having seen them at some point.  Not sure what's up
> there.

Hm. That's with -Wformat-truncation=1 (which is what's added by -Wextra
afaics), I see a good chunk more with -Wformat-truncation=2 - but that's
not enabled by -Wall/extra, so I don't really see a problem right now?

- Andres


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers



^ permalink  raw  reply  [nested|flat] 13+ messages in thread

* Re: GCC 7 warnings
  2017-04-10 15:03 GCC 7 warnings Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
@ 2017-04-10 16:25 ` Tom Lane <tgl@sss.pgh.pa.us>
  2017-04-11 17:57   ` Re: GCC 7 warnings Alvaro Herrera <alvherre@2ndquadrant.com>
  3 siblings, 1 reply; 13+ messages in thread

From: Tom Lane @ 2017-04-10 16:25 UTC (permalink / raw)
  To: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>; +Cc: pgsql-hackers

Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:
> Possible fixes:

> a) Ignore, hoping GCC will change before final release. (unlikely at
> this point)

> b) Add compiler option to disable this particular warning, worry about
> it later.  (Might be an option for backpatching.)

> c) Expand the target buffer sizes until the warning goes away.  (Sample
> patch attached.)

> d) Replace most of the problematic code with psprintf() and dynamically
> sized buffers.

+1 for (c) as you have it.  Later we might think about selectively
doing (d), but it seems like more work for probably not much benefit.

			regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers



^ permalink  raw  reply  [nested|flat] 13+ messages in thread

* Re: GCC 7 warnings
  2017-04-10 15:03 GCC 7 warnings Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
  2017-04-10 16:25 ` Re: GCC 7 warnings Tom Lane <tgl@sss.pgh.pa.us>
@ 2017-04-11 17:57   ` Alvaro Herrera <alvherre@2ndquadrant.com>
  2017-04-12 03:46     ` Re: GCC 7 warnings Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
  0 siblings, 1 reply; 13+ messages in thread

From: Alvaro Herrera @ 2017-04-11 17:57 UTC (permalink / raw)
  To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>; pgsql-hackers

Tom Lane wrote:
> Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:

> > d) Replace most of the problematic code with psprintf() and dynamically
> > sized buffers.
> 
> +1 for (c) as you have it.  Later we might think about selectively
> doing (d), but it seems like more work for probably not much benefit.

Yeah -- also it's possible some of these code paths must not attempt to
palloc() for robustness reasons.  I would go for c) only for now, and
only try d) for very specific cases where there are no such concerns.

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers



^ permalink  raw  reply  [nested|flat] 13+ messages in thread

* Re: GCC 7 warnings
  2017-04-10 15:03 GCC 7 warnings Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
  2017-04-10 16:25 ` Re: GCC 7 warnings Tom Lane <tgl@sss.pgh.pa.us>
  2017-04-11 17:57   ` Re: GCC 7 warnings Alvaro Herrera <alvherre@2ndquadrant.com>
@ 2017-04-12 03:46     ` Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
  2017-04-12 04:12       ` Re: GCC 7 warnings Tom Lane <tgl@sss.pgh.pa.us>
  0 siblings, 1 reply; 13+ messages in thread

From: Peter Eisentraut @ 2017-04-12 03:46 UTC (permalink / raw)
  To: Alvaro Herrera <alvherre@2ndquadrant.com>; Tom Lane <tgl@sss.pgh.pa.us>; +Cc: pgsql-hackers

On 4/11/17 13:57, Alvaro Herrera wrote:
> Tom Lane wrote:
>> Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:
> 
>>> d) Replace most of the problematic code with psprintf() and dynamically
>>> sized buffers.
>>
>> +1 for (c) as you have it.  Later we might think about selectively
>> doing (d), but it seems like more work for probably not much benefit.
> 
> Yeah -- also it's possible some of these code paths must not attempt to
> palloc() for robustness reasons.  I would go for c) only for now, and
> only try d) for very specific cases where there are no such concerns.

Attached is a more refined patch that I propose for PG10 now.  Compared
to the previous rushed version, this one uses some more precise
arithmetic to size some of the buffers.

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Attachments:

v2-0001-Fix-new-warnings-from-GCC-7.patch (invalid/octet-stream, 24.2K) download From 6fa0fe7f95d48bcd155e5f83026eb8fc3266d2ec Mon Sep 17 00:00:00 2001 From: Peter Eisentraut <peter_e@gmx.net> Date: Tue, 11 Apr 2017 14:13:31 -0400 Subject: [PATCH v2] Fix new warnings from GCC 7 This addresses the new warning types -Wformat-truncation -Wformat-overflow that are part of -Wall, via -Wformat, in GCC 7. --- contrib/pg_standby/pg_standby.c | 6 +++--- src/backend/access/heap/rewriteheap.c | 4 ++-- src/backend/access/transam/xlog.c | 6 +++--- src/backend/postmaster/pgstat.c | 4 ++-- src/backend/replication/basebackup.c | 4 ++-- src/backend/replication/logical/reorderbuffer.c | 2 +- src/backend/replication/logical/snapbuild.c | 4 ++-- src/backend/replication/slot.c | 6 +++--- src/backend/storage/file/copydir.c | 10 +++++----- src/backend/storage/file/fd.c | 12 ++++++------ src/backend/storage/file/reinit.c | 8 ++++---- src/backend/storage/ipc/dsm.c | 4 ++-- src/backend/utils/adt/dbsize.c | 14 +++++++------- src/backend/utils/adt/genfile.c | 4 ++-- src/backend/utils/cache/relcache.c | 4 ++-- src/backend/utils/error/elog.c | 2 +- src/backend/utils/time/snapmgr.c | 4 ++-- src/bin/pg_archivecleanup/pg_archivecleanup.c | 4 ++-- src/bin/pg_basebackup/pg_receivewal.c | 4 ++-- src/bin/pg_resetwal/pg_resetwal.c | 12 ++++++------ src/bin/pg_rewind/copy_fetch.c | 10 +++++----- src/common/file_utils.c | 4 ++-- src/timezone/pgtz.c | 4 ++-- 23 files changed, 68 insertions(+), 68 deletions(-) diff --git a/contrib/pg_standby/pg_standby.c b/contrib/pg_standby/pg_standby.c index e4d057e18e..5703032397 100644 --- a/contrib/pg_standby/pg_standby.c +++ b/contrib/pg_standby/pg_standby.c @@ -57,7 +57,7 @@ char *xlogFilePath; /* where we are going to restore to */ char *nextWALFileName; /* the file we need to get from archive */ char *restartWALFileName; /* the file from which we can restart restore */ char *priorWALFileName; /* the file we need to get from archive */ -char WALFilePath[MAXPGPATH]; /* the file path including archive */ +char WALFilePath[MAXPGPATH * 2]; /* the file path including archive */ char restoreCommand[MAXPGPATH]; /* run this to restore */ char exclusiveCleanupFileName[MAXFNAMELEN]; /* the file we need to * get from archive */ @@ -259,9 +259,9 @@ CustomizableCleanupPriorWALFiles(void) strcmp(xlde->d_name + 8, exclusiveCleanupFileName + 8) < 0) { #ifdef WIN32 - snprintf(WALFilePath, MAXPGPATH, "%s\\%s", archiveLocation, xlde->d_name); + snprintf(WALFilePath, sizeof(WALFilePath), "%s\\%s", archiveLocation, xlde->d_name); #else - snprintf(WALFilePath, MAXPGPATH, "%s/%s", archiveLocation, xlde->d_name); + snprintf(WALFilePath, sizeof(WALFilePath), "%s/%s", archiveLocation, xlde->d_name); #endif if (debug) diff --git a/src/backend/access/heap/rewriteheap.c b/src/backend/access/heap/rewriteheap.c index d7f65a5e99..60dcb67a20 100644 --- a/src/backend/access/heap/rewriteheap.c +++ b/src/backend/access/heap/rewriteheap.c @@ -1203,7 +1203,7 @@ CheckPointLogicalRewriteHeap(void) XLogRecPtr redo; DIR *mappings_dir; struct dirent *mapping_de; - char path[MAXPGPATH]; + char path[MAXPGPATH + 20]; /* * We start of with a minimum of the last redo pointer. No new decoding @@ -1234,7 +1234,7 @@ CheckPointLogicalRewriteHeap(void) strcmp(mapping_de->d_name, "..") == 0) continue; - snprintf(path, MAXPGPATH, "pg_logical/mappings/%s", mapping_de->d_name); + snprintf(path, sizeof(path), "pg_logical/mappings/%s", mapping_de->d_name); if (lstat(path, &statbuf) == 0 && !S_ISREG(statbuf.st_mode)) continue; diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index 45ed58ea34..c7667879c6 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -4102,7 +4102,7 @@ CleanupBackupHistory(void) { DIR *xldir; struct dirent *xlde; - char path[MAXPGPATH]; + char path[MAXPGPATH + sizeof(XLOGDIR)]; xldir = AllocateDir(XLOGDIR); if (xldir == NULL) @@ -4120,7 +4120,7 @@ CleanupBackupHistory(void) ereport(DEBUG2, (errmsg("removing transaction log backup history file \"%s\"", xlde->d_name))); - snprintf(path, MAXPGPATH, XLOGDIR "/%s", xlde->d_name); + snprintf(path, sizeof(path), XLOGDIR "/%s", xlde->d_name); unlink(path); XLogArchiveCleanup(xlde->d_name); } @@ -10389,7 +10389,7 @@ do_pg_start_backup(const char *backupidstr, bool fast, TimeLineID *starttli_p, /* Collect information about all tablespaces */ while ((de = ReadDir(tblspcdir, "pg_tblspc")) != NULL) { - char fullpath[MAXPGPATH]; + char fullpath[MAXPGPATH + 10]; char linkpath[MAXPGPATH]; char *relpath = NULL; int rllen; diff --git a/src/backend/postmaster/pgstat.c b/src/backend/postmaster/pgstat.c index 3fb57f060c..496e6aedde 100644 --- a/src/backend/postmaster/pgstat.c +++ b/src/backend/postmaster/pgstat.c @@ -606,7 +606,7 @@ pgstat_reset_remove_files(const char *directory) { DIR *dir; struct dirent *entry; - char fname[MAXPGPATH]; + char fname[MAXPGPATH * 2]; dir = AllocateDir(directory); while ((entry = ReadDir(dir, directory)) != NULL) @@ -636,7 +636,7 @@ pgstat_reset_remove_files(const char *directory) strcmp(entry->d_name + nchars, "stat") != 0) continue; - snprintf(fname, MAXPGPATH, "%s/%s", directory, + snprintf(fname, sizeof(fname), "%s/%s", directory, entry->d_name); unlink(fname); } diff --git a/src/backend/replication/basebackup.c b/src/backend/replication/basebackup.c index e3a7ad5e9a..3ee0dd5aa4 100644 --- a/src/backend/replication/basebackup.c +++ b/src/backend/replication/basebackup.c @@ -959,7 +959,7 @@ sendDir(char *path, int basepathlen, bool sizeonly, List *tablespaces, { DIR *dir; struct dirent *de; - char pathbuf[MAXPGPATH]; + char pathbuf[MAXPGPATH * 2]; struct stat statbuf; int64 size = 0; @@ -1011,7 +1011,7 @@ sendDir(char *path, int basepathlen, bool sizeonly, List *tablespaces, if (excludeFound) continue; - snprintf(pathbuf, MAXPGPATH, "%s/%s", path, de->d_name); + snprintf(pathbuf, sizeof(pathbuf), "%s/%s", path, de->d_name); /* Skip pg_control here to back up it last */ if (strcmp(pathbuf, "./global/pg_control") == 0) diff --git a/src/backend/replication/logical/reorderbuffer.c b/src/backend/replication/logical/reorderbuffer.c index 12ebadca81..0c174be8ed 100644 --- a/src/backend/replication/logical/reorderbuffer.c +++ b/src/backend/replication/logical/reorderbuffer.c @@ -2613,7 +2613,7 @@ StartupReorderBuffer(void) while ((logical_de = ReadDir(logical_dir, "pg_replslot")) != NULL) { struct stat statbuf; - char path[MAXPGPATH]; + char path[MAXPGPATH * 2 + 12]; if (strcmp(logical_de->d_name, ".") == 0 || strcmp(logical_de->d_name, "..") == 0) diff --git a/src/backend/replication/logical/snapbuild.c b/src/backend/replication/logical/snapbuild.c index 227960452d..358ec28932 100644 --- a/src/backend/replication/logical/snapbuild.c +++ b/src/backend/replication/logical/snapbuild.c @@ -1868,7 +1868,7 @@ CheckPointSnapBuild(void) XLogRecPtr redo; DIR *snap_dir; struct dirent *snap_de; - char path[MAXPGPATH]; + char path[MAXPGPATH + 21]; /* * We start off with a minimum of the last redo pointer. No new replication @@ -1895,7 +1895,7 @@ CheckPointSnapBuild(void) strcmp(snap_de->d_name, "..") == 0) continue; - snprintf(path, MAXPGPATH, "pg_logical/snapshots/%s", snap_de->d_name); + snprintf(path, sizeof(path), "pg_logical/snapshots/%s", snap_de->d_name); if (lstat(path, &statbuf) == 0 && !S_ISREG(statbuf.st_mode)) { diff --git a/src/backend/replication/slot.c b/src/backend/replication/slot.c index 6ac0ce69c6..e8ad0f7b39 100644 --- a/src/backend/replication/slot.c +++ b/src/backend/replication/slot.c @@ -1023,13 +1023,13 @@ StartupReplicationSlots(void) while ((replication_de = ReadDir(replication_dir, "pg_replslot")) != NULL) { struct stat statbuf; - char path[MAXPGPATH]; + char path[MAXPGPATH + 12]; if (strcmp(replication_de->d_name, ".") == 0 || strcmp(replication_de->d_name, "..") == 0) continue; - snprintf(path, MAXPGPATH, "pg_replslot/%s", replication_de->d_name); + snprintf(path, sizeof(path), "pg_replslot/%s", replication_de->d_name); /* we're only creating directories here, skip if it's not our's */ if (lstat(path, &statbuf) == 0 && !S_ISDIR(statbuf.st_mode)) @@ -1259,7 +1259,7 @@ RestoreSlotFromDisk(const char *name) { ReplicationSlotOnDisk cp; int i; - char path[MAXPGPATH]; + char path[MAXPGPATH + 22]; int fd; bool restored = false; int readBytes; diff --git a/src/backend/storage/file/copydir.c b/src/backend/storage/file/copydir.c index dffe28376b..1e2691685a 100644 --- a/src/backend/storage/file/copydir.c +++ b/src/backend/storage/file/copydir.c @@ -38,8 +38,8 @@ copydir(char *fromdir, char *todir, bool recurse) { DIR *xldir; struct dirent *xlde; - char fromfile[MAXPGPATH]; - char tofile[MAXPGPATH]; + char fromfile[MAXPGPATH * 2]; + char tofile[MAXPGPATH * 2]; if (mkdir(todir, S_IRWXU) != 0) ereport(ERROR, @@ -63,8 +63,8 @@ copydir(char *fromdir, char *todir, bool recurse) strcmp(xlde->d_name, "..") == 0) continue; - snprintf(fromfile, MAXPGPATH, "%s/%s", fromdir, xlde->d_name); - snprintf(tofile, MAXPGPATH, "%s/%s", todir, xlde->d_name); + snprintf(fromfile, sizeof(fromfile), "%s/%s", fromdir, xlde->d_name); + snprintf(tofile, sizeof(tofile), "%s/%s", todir, xlde->d_name); if (lstat(fromfile, &fst) < 0) ereport(ERROR, @@ -103,7 +103,7 @@ copydir(char *fromdir, char *todir, bool recurse) strcmp(xlde->d_name, "..") == 0) continue; - snprintf(tofile, MAXPGPATH, "%s/%s", todir, xlde->d_name); + snprintf(tofile, sizeof(tofile), "%s/%s", todir, xlde->d_name); /* * We don't need to sync subdirectories here since the recursive diff --git a/src/backend/storage/file/fd.c b/src/backend/storage/file/fd.c index b14979496c..81d96a4cc0 100644 --- a/src/backend/storage/file/fd.c +++ b/src/backend/storage/file/fd.c @@ -2705,7 +2705,7 @@ CleanupTempFiles(bool isProcExit) void RemovePgTempFiles(void) { - char temp_path[MAXPGPATH]; + char temp_path[MAXPGPATH + 10 + sizeof(TABLESPACE_VERSION_DIRECTORY) + sizeof(PG_TEMP_FILES_DIR)]; DIR *spc_dir; struct dirent *spc_de; @@ -2753,7 +2753,7 @@ RemovePgTempFilesInDir(const char *tmpdirname) { DIR *temp_dir; struct dirent *temp_de; - char rm_path[MAXPGPATH]; + char rm_path[MAXPGPATH * 2]; temp_dir = AllocateDir(tmpdirname); if (temp_dir == NULL) @@ -2794,7 +2794,7 @@ RemovePgTempRelationFiles(const char *tsdirname) { DIR *ts_dir; struct dirent *de; - char dbspace_path[MAXPGPATH]; + char dbspace_path[MAXPGPATH * 2]; ts_dir = AllocateDir(tsdirname); if (ts_dir == NULL) @@ -2835,7 +2835,7 @@ RemovePgTempRelationFilesInDbspace(const char *dbspacedirname) { DIR *dbspace_dir; struct dirent *de; - char rm_path[MAXPGPATH]; + char rm_path[MAXPGPATH * 2]; dbspace_dir = AllocateDir(dbspacedirname); if (dbspace_dir == NULL) @@ -3022,7 +3022,7 @@ walkdir(const char *path, while ((de = ReadDirExtended(dir, path, elevel)) != NULL) { - char subpath[MAXPGPATH]; + char subpath[MAXPGPATH * 2]; struct stat fst; int sret; @@ -3032,7 +3032,7 @@ walkdir(const char *path, strcmp(de->d_name, "..") == 0) continue; - snprintf(subpath, MAXPGPATH, "%s/%s", path, de->d_name); + snprintf(subpath, sizeof(subpath), "%s/%s", path, de->d_name); if (process_symlinks) sret = stat(subpath, &fst); diff --git a/src/backend/storage/file/reinit.c b/src/backend/storage/file/reinit.c index b883dfc657..f331e7bc21 100644 --- a/src/backend/storage/file/reinit.c +++ b/src/backend/storage/file/reinit.c @@ -48,7 +48,7 @@ typedef struct void ResetUnloggedRelations(int op) { - char temp_path[MAXPGPATH]; + char temp_path[MAXPGPATH + 10 + sizeof(TABLESPACE_VERSION_DIRECTORY)]; DIR *spc_dir; struct dirent *spc_de; MemoryContext tmpctx, @@ -104,7 +104,7 @@ ResetUnloggedRelationsInTablespaceDir(const char *tsdirname, int op) { DIR *ts_dir; struct dirent *de; - char dbspace_path[MAXPGPATH]; + char dbspace_path[MAXPGPATH * 2]; ts_dir = AllocateDir(tsdirname); if (ts_dir == NULL) @@ -145,7 +145,7 @@ ResetUnloggedRelationsInDbspaceDir(const char *dbspacedirname, int op) { DIR *dbspace_dir; struct dirent *de; - char rm_path[MAXPGPATH]; + char rm_path[MAXPGPATH * 2]; /* Caller must specify at least one operation. */ Assert((op & (UNLOGGED_RELATION_CLEANUP | UNLOGGED_RELATION_INIT)) != 0); @@ -308,7 +308,7 @@ ResetUnloggedRelationsInDbspaceDir(const char *dbspacedirname, int op) ForkNumber forkNum; int oidchars; char oidbuf[OIDCHARS + 1]; - char srcpath[MAXPGPATH]; + char srcpath[MAXPGPATH * 2]; char dstpath[MAXPGPATH]; /* Skip anything that doesn't look like a relation data file. */ diff --git a/src/backend/storage/ipc/dsm.c b/src/backend/storage/ipc/dsm.c index b65b63b721..387849e22c 100644 --- a/src/backend/storage/ipc/dsm.c +++ b/src/backend/storage/ipc/dsm.c @@ -307,9 +307,9 @@ dsm_cleanup_for_mmap(void) if (strncmp(dent->d_name, PG_DYNSHMEM_MMAP_FILE_PREFIX, strlen(PG_DYNSHMEM_MMAP_FILE_PREFIX)) == 0) { - char buf[MAXPGPATH]; + char buf[MAXPGPATH + sizeof(PG_DYNSHMEM_DIR)]; - snprintf(buf, MAXPGPATH, PG_DYNSHMEM_DIR "/%s", dent->d_name); + snprintf(buf, sizeof(buf), PG_DYNSHMEM_DIR "/%s", dent->d_name); elog(DEBUG2, "removing file \"%s\"", buf); diff --git a/src/backend/utils/adt/dbsize.c b/src/backend/utils/adt/dbsize.c index 6d56638208..b0418b18dc 100644 --- a/src/backend/utils/adt/dbsize.c +++ b/src/backend/utils/adt/dbsize.c @@ -41,7 +41,7 @@ db_dir_size(const char *path) int64 dirsize = 0; struct dirent *direntry; DIR *dirdesc; - char filename[MAXPGPATH]; + char filename[MAXPGPATH * 2]; dirdesc = AllocateDir(path); @@ -58,7 +58,7 @@ db_dir_size(const char *path) strcmp(direntry->d_name, "..") == 0) continue; - snprintf(filename, MAXPGPATH, "%s/%s", path, direntry->d_name); + snprintf(filename, sizeof(filename), "%s/%s", path, direntry->d_name); if (stat(filename, &fst) < 0) { @@ -86,7 +86,7 @@ calculate_database_size(Oid dbOid) DIR *dirdesc; struct dirent *direntry; char dirpath[MAXPGPATH]; - char pathname[MAXPGPATH]; + char pathname[MAXPGPATH + 12 + sizeof(TABLESPACE_VERSION_DIRECTORY)]; AclResult aclresult; /* @@ -104,7 +104,7 @@ calculate_database_size(Oid dbOid) /* Shared storage in pg_global is not counted */ /* Include pg_default storage */ - snprintf(pathname, MAXPGPATH, "base/%u", dbOid); + snprintf(pathname, sizeof(pathname), "base/%u", dbOid); totalsize = db_dir_size(pathname); /* Scan the non-default tablespaces */ @@ -124,7 +124,7 @@ calculate_database_size(Oid dbOid) strcmp(direntry->d_name, "..") == 0) continue; - snprintf(pathname, MAXPGPATH, "pg_tblspc/%s/%s/%u", + snprintf(pathname, sizeof(pathname), "pg_tblspc/%s/%s/%u", direntry->d_name, TABLESPACE_VERSION_DIRECTORY, dbOid); totalsize += db_dir_size(pathname); } @@ -172,7 +172,7 @@ static int64 calculate_tablespace_size(Oid tblspcOid) { char tblspcPath[MAXPGPATH]; - char pathname[MAXPGPATH]; + char pathname[MAXPGPATH * 2]; int64 totalsize = 0; DIR *dirdesc; struct dirent *direntry; @@ -215,7 +215,7 @@ calculate_tablespace_size(Oid tblspcOid) strcmp(direntry->d_name, "..") == 0) continue; - snprintf(pathname, MAXPGPATH, "%s/%s", tblspcPath, direntry->d_name); + snprintf(pathname, sizeof(pathname), "%s/%s", tblspcPath, direntry->d_name); if (stat(pathname, &fst) < 0) { diff --git a/src/backend/utils/adt/genfile.c b/src/backend/utils/adt/genfile.c index 8d0a236e6d..32d6a66688 100644 --- a/src/backend/utils/adt/genfile.c +++ b/src/backend/utils/adt/genfile.c @@ -522,7 +522,7 @@ pg_ls_dir_files(FunctionCallInfo fcinfo, char *dir) { Datum values[3]; bool nulls[3]; - char path[MAXPGPATH]; + char path[MAXPGPATH * 2]; struct stat attrib; HeapTuple tuple; @@ -531,7 +531,7 @@ pg_ls_dir_files(FunctionCallInfo fcinfo, char *dir) continue; /* Get the file info */ - snprintf(path, MAXPGPATH, "%s/%s", fctx->location, de->d_name); + snprintf(path, sizeof(path), "%s/%s", fctx->location, de->d_name); if (stat(path, &attrib) < 0) ereport(ERROR, (errcode_for_file_access(), diff --git a/src/backend/utils/cache/relcache.c b/src/backend/utils/cache/relcache.c index ddb948528b..5bf02d4c0e 100644 --- a/src/backend/utils/cache/relcache.c +++ b/src/backend/utils/cache/relcache.c @@ -6074,7 +6074,7 @@ RelationCacheInitFileRemove(void) const char *tblspcdir = "pg_tblspc"; DIR *dir; struct dirent *de; - char path[MAXPGPATH]; + char path[MAXPGPATH + 10 + sizeof(TABLESPACE_VERSION_DIRECTORY)]; /* * We zap the shared cache file too. In theory it can't get out of sync @@ -6116,7 +6116,7 @@ RelationCacheInitFileRemoveInDir(const char *tblspcpath) { DIR *dir; struct dirent *de; - char initfilename[MAXPGPATH]; + char initfilename[MAXPGPATH * 2]; /* Scan the tablespace directory to find per-database directories */ dir = AllocateDir(tblspcpath); diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.c index 6e83cbedab..5169714817 100644 --- a/src/backend/utils/error/elog.c +++ b/src/backend/utils/error/elog.c @@ -2228,7 +2228,7 @@ static void setup_formatted_log_time(void) { pg_time_t stamp_time; - char msbuf[8]; + char msbuf[10]; if (!saved_timeval_set) { diff --git a/src/backend/utils/time/snapmgr.c b/src/backend/utils/time/snapmgr.c index 3d92494e0b..3a3a25a0c3 100644 --- a/src/backend/utils/time/snapmgr.c +++ b/src/backend/utils/time/snapmgr.c @@ -1578,7 +1578,7 @@ XactHasExportedSnapshots(void) void DeleteAllExportedSnapshotFiles(void) { - char buf[MAXPGPATH]; + char buf[MAXPGPATH + sizeof(SNAPSHOT_EXPORT_DIR)]; DIR *s_dir; struct dirent *s_de; @@ -1599,7 +1599,7 @@ DeleteAllExportedSnapshotFiles(void) strcmp(s_de->d_name, "..") == 0) continue; - snprintf(buf, MAXPGPATH, SNAPSHOT_EXPORT_DIR "/%s", s_de->d_name); + snprintf(buf, sizeof(buf), SNAPSHOT_EXPORT_DIR "/%s", s_de->d_name); /* Again, unlink failure is not worthy of FATAL */ if (unlink(buf)) elog(LOG, "could not unlink file \"%s\": %m", buf); diff --git a/src/bin/pg_archivecleanup/pg_archivecleanup.c b/src/bin/pg_archivecleanup/pg_archivecleanup.c index f1651d4273..1760955586 100644 --- a/src/bin/pg_archivecleanup/pg_archivecleanup.c +++ b/src/bin/pg_archivecleanup/pg_archivecleanup.c @@ -32,7 +32,7 @@ char *additional_ext = NULL; /* Extension to remove from filenames */ char *archiveLocation; /* where to find the archive? */ char *restartWALFileName; /* the file from which we can restart restore */ -char WALFilePath[MAXPGPATH]; /* the file path including archive */ +char WALFilePath[MAXPGPATH * 2]; /* the file path including archive */ char exclusiveCleanupFileName[MAXFNAMELEN]; /* the oldest file we * want to remain in * archive */ @@ -133,7 +133,7 @@ CleanupPriorWALFiles(void) * extension that might have been chopped off before testing * the sequence. */ - snprintf(WALFilePath, MAXPGPATH, "%s/%s", + snprintf(WALFilePath, sizeof(WALFilePath), "%s/%s", archiveLocation, xlde->d_name); if (dryrun) diff --git a/src/bin/pg_basebackup/pg_receivewal.c b/src/bin/pg_basebackup/pg_receivewal.c index 15348ada58..1a9fe81be1 100644 --- a/src/bin/pg_basebackup/pg_receivewal.c +++ b/src/bin/pg_basebackup/pg_receivewal.c @@ -244,7 +244,7 @@ FindStreamingStart(uint32 *tli) if (!ispartial && !iscompress) { struct stat statbuf; - char fullpath[MAXPGPATH]; + char fullpath[MAXPGPATH * 2]; snprintf(fullpath, sizeof(fullpath), "%s/%s", basedir, dirent->d_name); if (stat(fullpath, &statbuf) != 0) @@ -267,7 +267,7 @@ FindStreamingStart(uint32 *tli) int fd; char buf[4]; int bytes_out; - char fullpath[MAXPGPATH]; + char fullpath[MAXPGPATH * 2]; snprintf(fullpath, sizeof(fullpath), "%s/%s", basedir, dirent->d_name); diff --git a/src/bin/pg_resetwal/pg_resetwal.c b/src/bin/pg_resetwal/pg_resetwal.c index bcb9ed938f..73bd9fe6ec 100644 --- a/src/bin/pg_resetwal/pg_resetwal.c +++ b/src/bin/pg_resetwal/pg_resetwal.c @@ -958,7 +958,7 @@ KillExistingXLOG(void) { DIR *xldir; struct dirent *xlde; - char path[MAXPGPATH]; + char path[MAXPGPATH + sizeof(XLOGDIR)]; xldir = opendir(XLOGDIR); if (xldir == NULL) @@ -973,7 +973,7 @@ KillExistingXLOG(void) if (IsXLogFileName(xlde->d_name) || IsPartialXLogFileName(xlde->d_name)) { - snprintf(path, MAXPGPATH, "%s/%s", XLOGDIR, xlde->d_name); + snprintf(path, sizeof(path), "%s/%s", XLOGDIR, xlde->d_name); if (unlink(path) < 0) { fprintf(stderr, _("%s: could not delete file \"%s\": %s\n"), @@ -1005,11 +1005,11 @@ KillExistingXLOG(void) static void KillExistingArchiveStatus(void) { +#define ARCHSTATDIR XLOGDIR "/archive_status" + DIR *xldir; struct dirent *xlde; - char path[MAXPGPATH]; - -#define ARCHSTATDIR XLOGDIR "/archive_status" + char path[MAXPGPATH + sizeof(ARCHSTATDIR)]; xldir = opendir(ARCHSTATDIR); if (xldir == NULL) @@ -1027,7 +1027,7 @@ KillExistingArchiveStatus(void) strcmp(xlde->d_name + XLOG_FNAME_LEN, ".partial.ready") == 0 || strcmp(xlde->d_name + XLOG_FNAME_LEN, ".partial.done") == 0)) { - snprintf(path, MAXPGPATH, "%s/%s", ARCHSTATDIR, xlde->d_name); + snprintf(path, sizeof(path), "%s/%s", ARCHSTATDIR, xlde->d_name); if (unlink(path) < 0) { fprintf(stderr, _("%s: could not delete file \"%s\": %s\n"), diff --git a/src/bin/pg_rewind/copy_fetch.c b/src/bin/pg_rewind/copy_fetch.c index f8d3dfecfa..347d4e4d4c 100644 --- a/src/bin/pg_rewind/copy_fetch.c +++ b/src/bin/pg_rewind/copy_fetch.c @@ -65,14 +65,14 @@ recurse_dir(const char *datadir, const char *parentpath, while (errno = 0, (xlde = readdir(xldir)) != NULL) { struct stat fst; - char fullpath[MAXPGPATH]; - char path[MAXPGPATH]; + char fullpath[MAXPGPATH * 2]; + char path[MAXPGPATH * 2]; if (strcmp(xlde->d_name, ".") == 0 || strcmp(xlde->d_name, "..") == 0) continue; - snprintf(fullpath, MAXPGPATH, "%s/%s", fullparentpath, xlde->d_name); + snprintf(fullpath, sizeof(fullpath), "%s/%s", fullparentpath, xlde->d_name); if (lstat(fullpath, &fst) < 0) { @@ -93,9 +93,9 @@ recurse_dir(const char *datadir, const char *parentpath, } if (parentpath) - snprintf(path, MAXPGPATH, "%s/%s", parentpath, xlde->d_name); + snprintf(path, sizeof(path), "%s/%s", parentpath, xlde->d_name); else - snprintf(path, MAXPGPATH, "%s", xlde->d_name); + snprintf(path, sizeof(path), "%s", xlde->d_name); if (S_ISREG(fst.st_mode)) callback(path, FILE_TYPE_REGULAR, fst.st_size, NULL); diff --git a/src/common/file_utils.c b/src/common/file_utils.c index 72b0565c71..218b83f391 100644 --- a/src/common/file_utils.c +++ b/src/common/file_utils.c @@ -166,7 +166,7 @@ walkdir(const char *path, while (errno = 0, (de = readdir(dir)) != NULL) { - char subpath[MAXPGPATH]; + char subpath[MAXPGPATH * 2]; struct stat fst; int sret; @@ -174,7 +174,7 @@ walkdir(const char *path, strcmp(de->d_name, "..") == 0) continue; - snprintf(subpath, MAXPGPATH, "%s/%s", path, de->d_name); + snprintf(subpath, sizeof(subpath), "%s/%s", path, de->d_name); if (process_symlinks) sret = stat(subpath, &fst); diff --git a/src/timezone/pgtz.c b/src/timezone/pgtz.c index 826857d943..b7cfc5f5ac 100644 --- a/src/timezone/pgtz.c +++ b/src/timezone/pgtz.c @@ -412,7 +412,7 @@ pg_tzenumerate_next(pg_tzenum *dir) while (dir->depth >= 0) { struct dirent *direntry; - char fullname[MAXPGPATH]; + char fullname[MAXPGPATH * 2]; struct stat statbuf; direntry = ReadDir(dir->dirdesc[dir->depth], dir->dirname[dir->depth]); @@ -429,7 +429,7 @@ pg_tzenumerate_next(pg_tzenum *dir) if (direntry->d_name[0] == '.') continue; - snprintf(fullname, MAXPGPATH, "%s/%s", + snprintf(fullname, sizeof(fullname), "%s/%s", dir->dirname[dir->depth], direntry->d_name); if (stat(fullname, &statbuf) != 0) ereport(ERROR, -- 2.12.2
^ permalink raw reply [nested|flat] 13+ messages in thread

* Re: GCC 7 warnings
  2017-04-10 15:03 GCC 7 warnings Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
  2017-04-10 16:25 ` Re: GCC 7 warnings Tom Lane <tgl@sss.pgh.pa.us>
  2017-04-11 17:57   ` Re: GCC 7 warnings Alvaro Herrera <alvherre@2ndquadrant.com>
  2017-04-12 03:46     ` Re: GCC 7 warnings Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
@ 2017-04-12 04:12       ` Tom Lane <tgl@sss.pgh.pa.us>
  2017-04-12 15:20         ` Re: GCC 7 warnings Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
  0 siblings, 1 reply; 13+ messages in thread

From: Tom Lane @ 2017-04-12 04:12 UTC (permalink / raw)
  To: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>; +Cc: Alvaro Herrera <alvherre@2ndquadrant.com>; pgsql-hackers

Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:
> Attached is a more refined patch that I propose for PG10 now.  Compared
> to the previous rushed version, this one uses some more precise
> arithmetic to size some of the buffers.

Generally +1 for switching the snprintf calls to use sizeof() rather
than repeating the declared sizes of the arrays.

The change in setup_formatted_log_time seems a bit weird:

-	char		msbuf[8];
+	char		msbuf[10];

The associated call is

	sprintf(msbuf, ".%03d", (int) (saved_timeval.tv_usec / 1000));

Now a human can see that saved_timeval.tv_usec must be 0..999999, so
that the %d format item must always emit exactly 3 characters, which
means that really 5 bytes would be enough.  I wouldn't expect a
compiler to know that, but if it's making a generic assumption about
the worst-case width of %d, shouldn't it conclude that we might need
as many as 13 bytes for the buffer?  Why does msbuf[10] satisfy it
if msbuf[8] doesn't?

IOW, if we're going to touch this at all, I'd be inclined to go with
msbuf[16] or so, as being more likely to satisfy compilers that have
decided to try to warn about this.  And maybe we should use snprintf,
just for luck.

			regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers



^ permalink  raw  reply  [nested|flat] 13+ messages in thread

* Re: GCC 7 warnings
  2017-04-10 15:03 GCC 7 warnings Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
  2017-04-10 16:25 ` Re: GCC 7 warnings Tom Lane <tgl@sss.pgh.pa.us>
  2017-04-11 17:57   ` Re: GCC 7 warnings Alvaro Herrera <alvherre@2ndquadrant.com>
  2017-04-12 03:46     ` Re: GCC 7 warnings Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
  2017-04-12 04:12       ` Re: GCC 7 warnings Tom Lane <tgl@sss.pgh.pa.us>
@ 2017-04-12 15:20         ` Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
  2017-04-12 15:31           ` Re: GCC 7 warnings Tom Lane <tgl@sss.pgh.pa.us>
  0 siblings, 1 reply; 13+ messages in thread

From: Peter Eisentraut @ 2017-04-12 15:20 UTC (permalink / raw)
  To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Alvaro Herrera <alvherre@2ndquadrant.com>; pgsql-hackers

On 4/12/17 00:12, Tom Lane wrote:
> The change in setup_formatted_log_time seems a bit weird:
> 
> -	char		msbuf[8];
> +	char		msbuf[10];
> 
> The associated call is
> 
> 	sprintf(msbuf, ".%03d", (int) (saved_timeval.tv_usec / 1000));
> 
> Now a human can see that saved_timeval.tv_usec must be 0..999999, so
> that the %d format item must always emit exactly 3 characters, which
> means that really 5 bytes would be enough.  I wouldn't expect a
> compiler to know that, but if it's making a generic assumption about
> the worst-case width of %d, shouldn't it conclude that we might need
> as many as 13 bytes for the buffer?  Why does msbuf[10] satisfy it
> if msbuf[8] doesn't?

Because the /1000 takes off three digits?

The full message from an isolated test case is

test.c: In function 'f':
test.c:11:15: warning: '%03d' directive writing between 3 and 8 bytes
into a region of size 7 [-Wformat-overflow=]
  sprintf(buf, ".%03d", (int) (tv.tv_usec / 1000));
               ^
test.c:11:15: note: directive argument in the range [-2147483, 2147483]
test.c:11:2: note: '__builtin___sprintf_chk' output between 5 and 10
bytes into a destination of size 8
  sprintf(buf, ".%03d", (int) (tv.tv_usec / 1000));
  ^

(This is with -O2.  With -O0 it only asks for 5 bytes.)

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers



^ permalink  raw  reply  [nested|flat] 13+ messages in thread

* Re: GCC 7 warnings
  2017-04-10 15:03 GCC 7 warnings Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
  2017-04-10 16:25 ` Re: GCC 7 warnings Tom Lane <tgl@sss.pgh.pa.us>
  2017-04-11 17:57   ` Re: GCC 7 warnings Alvaro Herrera <alvherre@2ndquadrant.com>
  2017-04-12 03:46     ` Re: GCC 7 warnings Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
  2017-04-12 04:12       ` Re: GCC 7 warnings Tom Lane <tgl@sss.pgh.pa.us>
  2017-04-12 15:20         ` Re: GCC 7 warnings Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
@ 2017-04-12 15:31           ` Tom Lane <tgl@sss.pgh.pa.us>
  0 siblings, 0 replies; 13+ messages in thread

From: Tom Lane @ 2017-04-12 15:31 UTC (permalink / raw)
  To: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>; +Cc: Alvaro Herrera <alvherre@2ndquadrant.com>; pgsql-hackers

Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:
> On 4/12/17 00:12, Tom Lane wrote:
>> Now a human can see that saved_timeval.tv_usec must be 0..999999, so
>> that the %d format item must always emit exactly 3 characters, which
>> means that really 5 bytes would be enough.  I wouldn't expect a
>> compiler to know that, but if it's making a generic assumption about
>> the worst-case width of %d, shouldn't it conclude that we might need
>> as many as 13 bytes for the buffer?  Why does msbuf[10] satisfy it
>> if msbuf[8] doesn't?

> Because the /1000 takes off three digits?

Huh ... I would not really have expected it to figure that out, but
this makes it pretty clear that it did:

> test.c:11:15: note: directive argument in the range [-2147483, 2147483]


> (This is with -O2.  With -O0 it only asks for 5 bytes.)

But that reinforces my suspicion that the intelligence brought to bear
here will be variable.  I'd still go for msbuf[16]; it's not like
anybody's going to notice the extra stack consumption.

			regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers



^ permalink  raw  reply  [nested|flat] 13+ messages in thread

* Re: GCC 7 warnings
  2017-04-10 15:03 GCC 7 warnings Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
@ 2017-05-04 01:59 ` Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
  2017-05-04 04:21   ` Re: GCC 7 warnings Tom Lane <tgl@sss.pgh.pa.us>
  3 siblings, 1 reply; 13+ messages in thread

From: Peter Eisentraut @ 2017-05-04 01:59 UTC (permalink / raw)
  To: pgsql-hackers

On 4/10/17 11:03, Peter Eisentraut wrote:
> The release of GCC 7 is approaching [0], and the number of warnings in
> PostgreSQL has gone up since we last looked [1].  Output attached.  (My
> version is 7.0.1 20170408.)

GCC 7 has been released.

Should we backpatch these warning fixes?  The commit in question is
6275f5d28a1577563f53f2171689d4f890a46881.  (I haven't actually checked
whether backpatches well.)

PG 9.2 and newer compile warning-free with GCC 6, so there would be some
value in preserving this with GCC 7.

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers



^ permalink  raw  reply  [nested|flat] 13+ messages in thread

* Re: GCC 7 warnings
  2017-04-10 15:03 GCC 7 warnings Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
  2017-05-04 01:59 ` Re: GCC 7 warnings Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
@ 2017-05-04 04:21   ` Tom Lane <tgl@sss.pgh.pa.us>
  2017-05-16 13:01     ` Re: GCC 7 warnings Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
  0 siblings, 1 reply; 13+ messages in thread

From: Tom Lane @ 2017-05-04 04:21 UTC (permalink / raw)
  To: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>; +Cc: pgsql-hackers

Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:
> On 4/10/17 11:03, Peter Eisentraut wrote:
>> The release of GCC 7 is approaching [0], and the number of warnings in
>> PostgreSQL has gone up since we last looked [1].  Output attached.  (My
>> version is 7.0.1 20170408.)

> GCC 7 has been released.

> Should we backpatch these warning fixes?  The commit in question is
> 6275f5d28a1577563f53f2171689d4f890a46881.  (I haven't actually checked
> whether backpatches well.)

Seems like that patch represents generally better coding practice,
and applying it would reduce cross-branch differences that would be
hazards for future patches, so I'm mostly +1 for this.

But I'd suggest waiting till after next week's releases.  If there
are any problems induced by this, we'd be more likely to find them
with another three months' time before it hits the wild.

			regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers



^ permalink  raw  reply  [nested|flat] 13+ messages in thread

* Re: GCC 7 warnings
  2017-04-10 15:03 GCC 7 warnings Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
  2017-05-04 01:59 ` Re: GCC 7 warnings Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
  2017-05-04 04:21   ` Re: GCC 7 warnings Tom Lane <tgl@sss.pgh.pa.us>
@ 2017-05-16 13:01     ` Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
  0 siblings, 0 replies; 13+ messages in thread

From: Peter Eisentraut @ 2017-05-16 13:01 UTC (permalink / raw)
  To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: pgsql-hackers

On 5/4/17 00:21, Tom Lane wrote:
> But I'd suggest waiting till after next week's releases.  If there
> are any problems induced by this, we'd be more likely to find them
> with another three months' time before it hits the wild.

done

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers



^ permalink  raw  reply  [nested|flat] 13+ messages in thread


end of thread, other threads:[~2017-05-16 13:01 UTC | newest]

Thread overview: 13+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2017-04-10 15:03 GCC 7 warnings Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
2017-04-10 15:35 ` Aleksander Alekseev <a.alekseev@postgrespro.ru>
2017-04-10 16:10 ` Andres Freund <andres@anarazel.de>
2017-04-10 16:16   ` Andres Freund <andres@anarazel.de>
2017-04-10 16:25 ` Tom Lane <tgl@sss.pgh.pa.us>
2017-04-11 17:57   ` Alvaro Herrera <alvherre@2ndquadrant.com>
2017-04-12 03:46     ` Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
2017-04-12 04:12       ` Tom Lane <tgl@sss.pgh.pa.us>
2017-04-12 15:20         ` Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
2017-04-12 15:31           ` Tom Lane <tgl@sss.pgh.pa.us>
2017-05-04 01:59 ` Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
2017-05-04 04:21   ` Tom Lane <tgl@sss.pgh.pa.us>
2017-05-16 13:01     ` Peter Eisentraut <peter.eisentraut@2ndquadrant.com>

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox