Thread

1. [PATCH] Move 'long long' check to c.h

   Stephen Frost <sfrost@snowman.net> — 2010-05-23T03:20:50Z

   Greetings,
   
     While reviewing bfba40e2c7b3909d3de13bd1b83b7e85fa8dfec2 (mmm, we like
     git diff -p), I noted that c.h is already included by both extern.h
     and ecpg.header through postgres_fe.h.  Given this and that we're
     already doing alot of similar #define's there (unlike in those other
     files), I felt c.h was a more appropriate place.  Putting it in c.h
     also means we don't have to duplicate that code.
   
     Patch attached.
   
     	Thanks,
   
   		Stephen
   

2. Re: [PATCH] Move 'long long' check to c.h

   Tom Lane <tgl@sss.pgh.pa.us> — 2010-05-23T15:24:17Z

   Stephen Frost <sfrost@snowman.net> writes:
   >   While reviewing bfba40e2c7b3909d3de13bd1b83b7e85fa8dfec2 (mmm, we like
   >   git diff -p), I noted that c.h is already included by both extern.h
   >   and ecpg.header through postgres_fe.h.  Given this and that we're
   >   already doing alot of similar #define's there (unlike in those other
   >   files), I felt c.h was a more appropriate place.  Putting it in c.h
   >   also means we don't have to duplicate that code.
   
   Ugh.  Moving that to c.h doesn't render it not junk code.  (For one
   thing, it will not operate as intended if you haven't previously
   #included <limits.h>, which in fact is not included in c.h.)
   
   If we need this we should do it properly with autoconf.
   
   			regards, tom lane
   
   

3. Re: [PATCH] Move 'long long' check to c.h

   Stephen Frost <sfrost@snowman.net> — 2010-05-23T15:50:00Z

   * Tom Lane (tgl@sss.pgh.pa.us) wrote:
   > Ugh.  Moving that to c.h doesn't render it not junk code.  (For one
   > thing, it will not operate as intended if you haven't previously
   > #included <limits.h>, which in fact is not included in c.h.)
   
   Doh.
   
   > If we need this we should do it properly with autoconf.
   
   My autoconf foo is not very good, but once I finish a couple of other
   things I'll take a shot at doing it that way.
   
   	Thanks!
   
   		Stephen
   

4. Re: [PATCH] Move 'long long' check to c.h

   Michael Meskes <meskes@postgresql.org> — 2010-05-24T16:13:55Z

   On Sat, May 22, 2010 at 11:20:50PM -0400, Stephen Frost wrote:
   >   git diff -p), I noted that c.h is already included by both extern.h
   >   and ecpg.header through postgres_fe.h.  Given this and that we're
   >   already doing alot of similar #define's there (unlike in those other
   >   files), I felt c.h was a more appropriate place.  Putting it in c.h
   >   also means we don't have to duplicate that code.
   
   But do other parts of PG also need it? Keep in mind that this works for ecpg
   because it needs LLONG_MIN or LONGLONG_MIN anyway. I'm not sure if there are
   compilers that have long long without those defines, but I'd guess there
   aren't.
   
   Michael
   -- 
   Michael Meskes
   Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org)
   Michael at BorussiaFan dot De, Meskes at (Debian|Postgresql) dot Org
   ICQ 179140304, AIM/Yahoo/Skype michaelmeskes, Jabber meskes@jabber.org
   VfL Borussia! Força Barça! Go SF 49ers! Use Debian GNU/Linux, PostgreSQL
   
   

5. Re: [PATCH] Move 'long long' check to c.h

   Michael Meskes <meskes@postgresql.org> — 2010-05-24T16:15:26Z

   On Sun, May 23, 2010 at 11:50:00AM -0400, Stephen Frost wrote:
   > > If we need this we should do it properly with autoconf.
   
   I absolutely agree and planed to do that *after* the release if it makes sense
   for the rest of PG, but wouldn't want to mess with it in the current
   situtation. On the other hand I didn't want to release with that bug in there.
   
   Michael
   -- 
   Michael Meskes
   Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org)
   Michael at BorussiaFan dot De, Meskes at (Debian|Postgresql) dot Org
   ICQ 179140304, AIM/Yahoo/Skype michaelmeskes, Jabber meskes@jabber.org
   VfL Borussia! Força Barça! Go SF 49ers! Use Debian GNU/Linux, PostgreSQL
   
   

6. Re: [PATCH] Move 'long long' check to c.h

   Tom Lane <tgl@sss.pgh.pa.us> — 2010-05-24T17:12:21Z

   Michael Meskes <meskes@postgresql.org> writes:
   > On Sat, May 22, 2010 at 11:20:50PM -0400, Stephen Frost wrote:
   >> git diff -p), I noted that c.h is already included by both extern.h
   >> and ecpg.header through postgres_fe.h.  Given this and that we're
   >> already doing alot of similar #define's there (unlike in those other
   >> files), I felt c.h was a more appropriate place.  Putting it in c.h
   >> also means we don't have to duplicate that code.
   
   > But do other parts of PG also need it? Keep in mind that this works for ecpg
   > because it needs LLONG_MIN or LONGLONG_MIN anyway. I'm not sure if there are
   > compilers that have long long without those defines, but I'd guess there
   > aren't.
   
   I think the current coding is extremely fragile (if it indeed works at
   all) because of its assumption that <limits.h> has been included
   already.  In any case, we have configure tests that exist only for the
   benefit of contrib modules, so it's hard to argue that we shouldn't have
   one that exists only for ecpg.
   
   I think we should fix this (properly) for 9.0.
   
   			regards, tom lane
   
   

7. Re: [PATCH] Move 'long long' check to c.h

   Michael Meskes <meskes@postgresql.org> — 2010-05-24T19:11:32Z

   > I think the current coding is extremely fragile (if it indeed works at
   > all) because of its assumption that <limits.h> has been included
   
   Well, this is the case in the code so far. 
   
   > already.  In any case, we have configure tests that exist only for the
   > benefit of contrib modules, so it's hard to argue that we shouldn't have
   > one that exists only for ecpg.
   > 
   > I think we should fix this (properly) for 9.0.
   
   Ok, I don't mind fixing it properly for 9.0. Will do so as soon as I find time.
   
   Michael
   
   -- 
   Michael Meskes
   Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org)
   Michael at BorussiaFan dot De, Meskes at (Debian|Postgresql) dot Org
   ICQ 179140304, AIM/Yahoo/Skype michaelmeskes, Jabber meskes@jabber.org
   VfL Borussia! Força Barça! Go SF 49ers! Use Debian GNU/Linux, PostgreSQL
   
   

8. Re: [PATCH] Move 'long long' check to c.h

   Stephen Frost <sfrost@snowman.net> — 2010-05-24T19:20:14Z

   * Michael Meskes (meskes@postgresql.org) wrote:
   > > I think the current coding is extremely fragile (if it indeed works at
   > > all) because of its assumption that <limits.h> has been included
   > 
   > Well, this is the case in the code so far. 
   
   Right, the existing code is after limits.h is included, my suggestion to
   put it in c.h would have lost limits.h and broken things.  Sorry about
   that.  I didn't realize the dependency and make check didn't complain
   (not that I'm sure there's even a way we could have a regression test
   for this..).  I didn't intend to imply the currently-committed code
   didn't work (I figured it was probably fine :), was just trying to tidy
   a bit.
   
   	Thanks!
   
   		Stephen