On 25.03.2013 15:36, Tom Lane wrote:
Heikki Linnakangas<heikki.linnakan...@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
*** a/src/bin/pg_dump/dumputils.c
--- b/src/bin/pg_dump/dumputils.c
***************
*** 38,44 **** static struct
} on_exit_nicely_list[MAX_ON_EXIT_NICELY];
static int on_exit_nicely_index;
- void (*on_exit_msg_func) (const char *modulename, const char *fmt, va_list ap) = vwrite_msg;
#define supports_grant_options(version) ((version) >= 70400)
--- 38,43 ----
***************
*** 1365,1386 **** vwrite_msg(const char *modulename, const char *fmt, va_list ap)
vfprintf(stderr, _(fmt), ap);
}
-
- /*
- * Fail and die, with a message to stderr. Parameters as for write_msg.
- */
- void
- exit_horribly(const char *modulename, const char *fmt,...)
- {
- va_list ap;
-
- va_start(ap, fmt);
- on_exit_msg_func(modulename, fmt, ap);
- va_end(ap);
-
- exit_nicely(1);
- }
-
/* Register a callback to be run when exit_nicely is invoked. */
void
on_exit_nicely(on_exit_nicely_callback function, void *arg)
--- 1364,1369 ----
*** a/src/bin/pg_dump/dumputils.h
--- b/src/bin/pg_dump/dumputils.h
***************
*** 44,51 **** typedef void (*on_exit_nicely_callback) (int code, void *arg);
extern int quote_all_identifiers;
extern const char *progname;
- extern void (*on_exit_msg_func) (const char *modulename, const char *fmt, va_list ap)
- __attribute__((format(PG_PRINTF_ATTRIBUTE, 2, 0)));
extern void init_parallel_dump_utils(void);
extern const char *fmtId(const char *identifier);
--- 44,49 ----
***************
*** 86,91 **** __attribute__((format(PG_PRINTF_ATTRIBUTE, 2, 3)));
--- 84,93 ----
extern void
vwrite_msg(const char *modulename, const char *fmt, va_list ap)
__attribute__((format(PG_PRINTF_ATTRIBUTE, 2, 0)));
+ /*
+ * exit_horribly is not implemented in dumputils.c. Rather, pg_dump and
+ * pg_dumpall have a different implementation of it, with the same signature.
+ */
extern void
exit_horribly(const char *modulename, const char *fmt,...)
__attribute__((format(PG_PRINTF_ATTRIBUTE, 2, 3), noreturn));
*** a/src/bin/pg_dump/parallel.c
--- b/src/bin/pg_dump/parallel.c
***************
*** 72,87 **** typedef struct ShutdownInformation
Archive *AHX;
} ShutdownInformation;
! static ShutdownInformation shutdown_info;
static const char *modulename = gettext_noop("parallel archiver");
static ParallelSlot *GetMyPSlot(ParallelState *pstate);
static void
- parallel_exit_msg_func(const char *modulename,
- const char *fmt, va_list ap)
- __attribute__((format(PG_PRINTF_ATTRIBUTE, 2, 0)));
- static void
parallel_msg_master(ParallelSlot *slot, const char *modulename,
const char *fmt, va_list ap)
__attribute__((format(PG_PRINTF_ATTRIBUTE, 3, 0)));
--- 72,83 ----
Archive *AHX;
} ShutdownInformation;
! static ShutdownInformation shutdown_info = { NULL, NULL };
static const char *modulename = gettext_noop("parallel archiver");
static ParallelSlot *GetMyPSlot(ParallelState *pstate);
static void
parallel_msg_master(ParallelSlot *slot, const char *modulename,
const char *fmt, va_list ap)
__attribute__((format(PG_PRINTF_ATTRIBUTE, 3, 0)));
***************
*** 129,157 **** GetMyPSlot(ParallelState *pstate)
}
/*
! * This is the function that will be called from exit_horribly() to print the
! * error message. If the worker process does exit_horribly(), we forward its
! * last words to the master process. The master process then does
! * exit_horribly() with this error message itself and prints it normally.
! * After printing the message, exit_horribly() on the master will shut down
! * the remaining worker processes.
*/
! static void
! parallel_exit_msg_func(const char *modulename, const char *fmt, va_list ap)
{
ParallelState *pstate = shutdown_info.pstate;
ParallelSlot *slot;
! Assert(pstate);
! slot = GetMyPSlot(pstate);
!
! if (!slot)
! /* We're the parent, just write the message out */
vwrite_msg(modulename, fmt, ap);
else
! /* If we're a worker process, send the msg to the master process */
! parallel_msg_master(slot, modulename, fmt, ap);
}
/* Sends the error message from the worker to the master process */
--- 125,168 ----
}
/*
! * Fail and die, with a message to stderr. Parameters as for write_msg.
! *
! * This is defined in parallel.c because in parallel mode, things are more
! * complicated. If The worker process does exit_horribly(), we forward its last
! * words to the master process. The master process then does exit_horribly()
! * with this error message itself and prints it normally. After printing the
! * message, exit_horribly() on the master will shut down the remaining worker
! * processes.
*/
! void
! exit_horribly(const char *modulename, const char *fmt,...)
{
+ va_list ap;
ParallelState *pstate = shutdown_info.pstate;
ParallelSlot *slot;
! va_start(ap, fmt);
! if (pstate == NULL)
! {
! /* Not in parallel mode, just write it out */
vwrite_msg(modulename, fmt, ap);
+ }
else
! {
! slot = GetMyPSlot(pstate);
!
! if (!slot)
! /* We're the parent, just write the message out */
! vwrite_msg(modulename, fmt, ap);
! else
! /* If we're a worker process, send the msg to the master process */
! parallel_msg_master(slot, modulename, fmt, ap);
! }
!
! va_end(ap);
!
! exit_nicely(1);
}
/* Sends the error message from the worker to the master process */
***************
*** 408,414 **** ParallelBackupStart(ArchiveHandle *AH, RestoreOptions *ropt)
* set and falls back to AHX otherwise.
*/
shutdown_info.pstate = pstate;
- on_exit_msg_func = parallel_exit_msg_func;
#ifdef WIN32
tMasterThreadId = GetCurrentThreadId();
--- 419,424 ----
*** a/src/bin/pg_dump/pg_dumpall.c
--- b/src/bin/pg_dump/pg_dumpall.c
***************
*** 583,588 **** help(void)
--- 583,602 ----
printf(_("Report bugs to <pgsql-b...@postgresql.org>.\n"));
}
+ /*
+ * Fail and die, with a message to stderr. Parameters as for write_msg.
+ */
+ void
+ exit_horribly(const char *modulename, const char *fmt,...)
+ {
+ va_list ap;
+
+ va_start(ap, fmt);
+ vwrite_msg(modulename, fmt, ap);
+ va_end(ap);
+
+ exit_nicely(1);
+ }
/*
* Drop roles
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
