pg_upgrade's exec_prog() coding improvement
От | Alvaro Herrera |
---|---|
Тема | pg_upgrade's exec_prog() coding improvement |
Дата | |
Msg-id | 1345749634-sup-8678@alvh.no-ip.org обсуждение исходный текст |
Ответы |
Re: pg_upgrade's exec_prog() coding improvement
|
Список | 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’ formatattribute [-Wmissing-format-attribute] /pgsql/source/HEAD/contrib/pg_upgrade/exec.c:96:2: warning: function might be possible candidate for ‘gnu_printf’ formatattribute [-Wmissing-format-attribute] I have no idea how to silence that. Ideas? -- Álvaro Herrera http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
Вложения
В списке pgsql-hackers по дате отправления: