Обсуждение: pgsql: Add PF_PRINTF_ATTRIBUTE to on_exit_msg_fmt.
Add PF_PRINTF_ATTRIBUTE to on_exit_msg_fmt. Per warning from -Wmissing-format-attribute. Branch ------ master Details ------- http://git.postgresql.org/pg/commitdiff/ea988ee8c8b191615e730f930bcde6144a598688 Modified Files -------------- src/bin/pg_dump/dumputils.h | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-)
Heikki Linnakangas <heikki.linnakangas@iki.fi> writes: > Add PF_PRINTF_ATTRIBUTE to on_exit_msg_fmt. > Per warning from -Wmissing-format-attribute. Hm, this is exactly what I removed yesterday, because it makes the build fail outright on old gcc: gcc -O1 -Wall -Wmissing-prototypes -Wpointer-arith -Wformat-security -fno-strict-aliasing -g -I../../../src/interfaces/libpq-I../../../src/include -D_XOPEN_SOURCE_EXTENDED -D_USE_CTYPE_MACROS -c -o pg_dump.o pg_dump.c In file included from pg_backup.h:29, from pg_backup_archiver.h:32, from pg_dump.c:60: dumputils.h:48: argument format specified for non-function `on_exit_msg_func' make: *** [pg_dump.o] Error 1 Perhaps we have to refactor to avoid the use of a function variable here. It didn't seem particularly critical to do it like that rather than with, say, a bool. Or maybe we should turn off that warning. It seems to be leaping to conclusions about what the usage of the function variable is. regards, tom lane
On 25.03.2013 15:36, Tom Lane wrote: > Heikki Linnakangas<heikki.linnakangas@iki.fi> writes: >> Add PF_PRINTF_ATTRIBUTE to on_exit_msg_fmt. >> Per warning from -Wmissing-format-attribute. > > Hm, this is exactly what I removed yesterday, because it makes the build > fail outright on old gcc: > > gcc -O1 -Wall -Wmissing-prototypes -Wpointer-arith -Wformat-security -fno-strict-aliasing -g -I../../../src/interfaces/libpq-I../../../src/include -D_XOPEN_SOURCE_EXTENDED -D_USE_CTYPE_MACROS -c -o pg_dump.o pg_dump.c > In file included from pg_backup.h:29, > from pg_backup_archiver.h:32, > from pg_dump.c:60: > dumputils.h:48: argument format specified for non-function `on_exit_msg_func' > make: *** [pg_dump.o] Error 1 > > Perhaps we have to refactor to avoid the use of a function variable > here. It didn't seem particularly critical to do it like that rather > than with, say, a bool. Hmm. exit_horribly() is also used in pg_dumpall, so if you just put a call to a function in parallel.c in there, the linker will complain when linking pg_dumpall. BTW, we never reset on_exit_msg_func, even after getting out of parallel mode by calling ParallelBackupEnd(). The Assert in parallel_exit_msg_func() will fail if it gets called after ParallelBackupEnd(). The attached seems to work. With this patch, on_exit_msg_func() is gone. There's a different implementation of exit_horribly for pg_dumpall and pg_dump/restore. In pg_dumpall, it just calls vwrite_msg(). In pg_dump/restore's version, the logic from parallel_exit_msg_func() is moved directly to exit_horribly(). > Or maybe we should turn off that warning. It seems to be leaping to > conclusions about what the usage of the function variable is. Oh? Its conclusion seems correct to me: the function variable takes printf-like arguments. - Heikki
Вложения
Heikki Linnakangas <hlinnakangas@vmware.com> writes: > On 25.03.2013 15:36, Tom Lane wrote: >> Heikki Linnakangas<heikki.linnakangas@iki.fi> writes: >>> Add PF_PRINTF_ATTRIBUTE to on_exit_msg_fmt. >>> Per warning from -Wmissing-format-attribute. >> Hm, this is exactly what I removed yesterday, because it makes the build >> fail outright on old gcc: > The attached seems to work. With this patch, on_exit_msg_func() is gone. > There's a different implementation of exit_horribly for pg_dumpall and > pg_dump/restore. In pg_dumpall, it just calls vwrite_msg(). In > pg_dump/restore's version, the logic from parallel_exit_msg_func() is > moved directly to exit_horribly(). Seems probably reasonable, though if we're taking exit_horribly out of dumputils.c, meseems it ought not be declared in dumputils.h anymore. Can we put that declaration someplace else, rather than commenting it with an apology? regards, tom lane