Thread: 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
Attachment
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