Re: [HACKERS] pg_upgrade's exec_prog() coding improvement

2012-08-31 Thread Andrew Dunstan


On 08/24/2012 11:44 AM, Alvaro Herrera wrote:


Again, win32 testing would be welcome.  Sadly, buildfarm does not run
pg_upgrade's make check.




Yesterday I added a new module to the buildfarm client code to run this 
(https://github.com/PGBuildFarm/client-code/commit/ab812cb9920c65e39ec7358dc816371f1fef31eb). 
It required a couple of tweaks in the base code. This will be in a new 
buildfarm client release fairly shortly. It's running on crake now, and 
I will add it to pitta to get some Windows coverage.


It would be a lot nicer is the test were written in Perl, since we don't 
necessarily have a Bourne shell available for MSVC builds, but we 
definitely have Perl available.


None of this does what I think we really need, which is cross-release 
pg_upgrade testing, which remains on my TODO list as a fairly high 
priority item.


cheers

andrew


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] pg_upgrade's exec_prog() coding improvement

2012-08-31 Thread Andrew Dunstan


On 08/31/2012 10:52 AM, Andrew Dunstan wrote:


On 08/24/2012 11:44 AM, Alvaro Herrera wrote:


Again, win32 testing would be welcome.  Sadly, buildfarm does not run
pg_upgrade's make check.




Yesterday I added a new module to the buildfarm client code to run 
this 
(https://github.com/PGBuildFarm/client-code/commit/ab812cb9920c65e39ec7358dc816371f1fef31eb). 
It required a couple of tweaks in the base code. This will be in a new 
buildfarm client release fairly shortly. It's running on crake now, 
and I will add it to pitta to get some Windows coverage.




but it's not working on pitta :-(

It will take me some time to work out why, and I have several more 
urgent things on my plate. Meanwhile at least we have Linux coverage.


cheers

andrew




--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] pg_upgrade's exec_prog() coding improvement

2012-08-27 Thread Alvaro Herrera
Excerpts from Alvaro Herrera's message of vie ago 24 11:44:33 -0400 2012:
 Actually it seems better to just get rid of the extra varargs function
 and just have a single exec_prog.  The extra NULL argument is not
 troublesome enough, it seems.  Updated version attached.

Applied (with some very minor changes).

-- 
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training  Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] pg_upgrade's exec_prog() coding improvement

2012-08-24 Thread Heikki Linnakangas

On 23.08.2012 23:07, Alvaro Herrera wrote:

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?


You can do what the warning suggests, and tell the compiler that 
exec_prog takes printf-like arguments. See elog.h for an example of that:


extern int
errmsg(const char *fmt,...)
/* This extension allows gcc to check the format string for consistency with
   the supplied arguments. */
__attribute__((format(PG_PRINTF_ATTRIBUTE, 1, 2)));

--
  Heikki Linnakangas
  EnterpriseDB   http://www.enterprisedb.com


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] pg_upgrade's exec_prog() coding improvement

2012-08-24 Thread Tom Lane
Heikki Linnakangas heikki.linnakan...@enterprisedb.com writes:
 On 23.08.2012 23:07, Alvaro Herrera wrote:
 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?

 You can do what the warning suggests, and tell the compiler that 
 exec_prog takes printf-like arguments.

exec_prog already has such decoration, and Alvaro's patch doesn't remove
it.  So the question is, exactly what the heck does that warning mean?

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] pg_upgrade's exec_prog() coding improvement

2012-08-24 Thread Bruce Momjian
On Fri, Aug 24, 2012 at 10:08:58AM -0400, Tom Lane wrote:
 Heikki Linnakangas heikki.linnakan...@enterprisedb.com writes:
  On 23.08.2012 23:07, Alvaro Herrera wrote:
  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?
 
  You can do what the warning suggests, and tell the compiler that 
  exec_prog takes printf-like arguments.
 
 exec_prog already has such decoration, and Alvaro's patch doesn't remove
 it.  So the question is, exactly what the heck does that warning mean?

It sounds like it is suggestioning to use more specific attribute
decoration.  This might be relevant:

http://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html

-Wmissing-format-attribute
Warn about function pointers that might be candidates for format
attributes. Note these are only possible candidates, not absolute ones.
GCC guesses that function pointers with format attributes that are used
in assignment, initialization, parameter passing or return statements
should have a corresponding format attribute in the resulting type. I.e.
the left-hand side of the assignment or initialization, the type of the
parameter variable, or the return type of the containing function
respectively should also have a format attribute to avoid the warning.

GCC also warns about function definitions that might be candidates
for format attributes. Again, these are only possible candidates. GCC
guesses that format attributes might be appropriate for any function
that calls a function like vprintf or vscanf, but this might not always
be the case, and some functions for which format attributes are
appropriate may not be detected. 

and I see this option enabled in configure.in.

-- 
  Bruce Momjian  br...@momjian.ushttp://momjian.us
  EnterpriseDB http://enterprisedb.com

  + It's impossible for everything to be true. +


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] pg_upgrade's exec_prog() coding improvement

2012-08-24 Thread Tom Lane
Bruce Momjian br...@momjian.us writes:
 It sounds like it is suggestioning to use more specific attribute
 decoration.  This might be relevant:

   http://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html

   -Wmissing-format-attribute
   Warn about function pointers that might be candidates for format
   attributes. Note these are only possible candidates, not absolute ones.
   GCC guesses that function pointers with format attributes that are used
   in assignment, initialization, parameter passing or return statements
   should have a corresponding format attribute in the resulting type. I.e.
   the left-hand side of the assignment or initialization, the type of the
   parameter variable, or the return type of the containing function
   respectively should also have a format attribute to avoid the warning.

   GCC also warns about function definitions that might be candidates
   for format attributes. Again, these are only possible candidates. GCC
   guesses that format attributes might be appropriate for any function
   that calls a function like vprintf or vscanf, but this might not always
   be the case, and some functions for which format attributes are
   appropriate may not be detected. 

 and I see this option enabled in configure.in.

Hm.  I'm a bit dubious about enabling warnings that are so admittedly
heuristic as this.  They admit that the warnings might be wrong, and
yet document no way to shut them up.  I think we might be better advised
to not enable this warning.

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] pg_upgrade's exec_prog() coding improvement

2012-08-24 Thread Alvaro Herrera
Actually it seems better to just get rid of the extra varargs function
and just have a single exec_prog.  The extra NULL argument is not
troublesome enough, it seems.  Updated version attached.

Again, win32 testing would be welcome.  Sadly, buildfarm does not run
pg_upgrade's make check.

-- 
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training  Services


upgrade_execprog-2.patch
Description: Binary data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] pg_upgrade's exec_prog() coding improvement

2012-08-24 Thread Peter Eisentraut
On Fri, 2012-08-24 at 10:08 -0400, Tom Lane wrote:
 Heikki Linnakangas heikki.linnakan...@enterprisedb.com writes:
  On 23.08.2012 23:07, Alvaro Herrera wrote:
  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?
 
  You can do what the warning suggests, and tell the compiler that 
  exec_prog takes printf-like arguments.
 
 exec_prog already has such decoration, and Alvaro's patch doesn't remove
 it.  So the question is, exactly what the heck does that warning mean?

The warning is about s_exec_prog, not exec_prog.



-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers