From: | Alvaro Herrera <alvherre(at)2ndquadrant(dot)com> |
---|---|
To: | Pg Hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | pg_upgrade's exec_prog() coding improvement |
Date: | 2012-08-23 20:07:02 |
Message-ID: | 1345749634-sup-8678@alvh.no-ip.org |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Hi,
I've been bitten twice by exec_prog()s API, so here's a patch to try to
make it a bit harder to misuse.
There are two main changes here; one is to put the logfile option as the
first argument; then comes a bool, then the format string. This means
you get a warning if you pass the wrong number of arguments before the
format; previously I mis-merged in the KEY SHARE patch so that I was
passing the log file as format, and the compiler didn't complain at all.
The other interesting change I did was move the responsibility of adding
SYSTEMQUOTE and the ">> %s 2>&1" redirections to exec_prog itself,
reducing clutter in the caller. This makes the callers a lot easier to
read.
One other minor change I did was split it in two versions: a simpler one
with less frammishes, that doesn't let you supply an alternative log
file and doesn't return a status value. This is used for all but one of
the callers; only that one caller was interested in the result value
anyway. And that caller is also the only one that passes an
opt_log_file other than NULL, so I removed that from the simple version.
Lastly, I removed the is_priv boolean, which seems pretty pointless;
just made all calls set the umask inconditionally. The only caller
doing this was the cp/xcopy call, and I don't see any reason for this to
be different from other callers.
This makes pg_upgrade a bit more readable.
If anyone can test this on Windows I would be appreciate it.
One problem with this is that I get this warning:
/pgsql/source/HEAD/contrib/pg_upgrade/exec.c: In function ‘s_exec_prog’:
/pgsql/source/HEAD/contrib/pg_upgrade/exec.c:96:2: warning: function might be possible candidate for ‘gnu_printf’ format attribute [-Wmissing-format-attribute]
/pgsql/source/HEAD/contrib/pg_upgrade/exec.c:96:2: warning: function might be possible candidate for ‘gnu_printf’ format attribute [-Wmissing-format-attribute]
I have no idea how to silence that. Ideas?
--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
Attachment | Content-Type | Size |
---|---|---|
upgrade_execprog.patch | application/octet-stream | 16.8 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Bruce Momjian | 2012-08-23 20:41:51 | Re: [WIP] Performance Improvement by reducing WAL for Update Operation |
Previous Message | Tom Lane | 2012-08-23 20:04:01 | Re: default_isolation_level='serializable' crashes on Windows |