Обсуждение: Re: [COMMITTERS] pgsql: Centralize definition of integer limits.
Michael Paquier <michael.paquier@gmail.com> writes:
> On Mon, Mar 30, 2015 at 7:48 PM, Andres Freund <andres@anarazel.de> wrote:
>> On 2015-03-30 14:01:25 +0900, Michael Paquier wrote:
>>> My OSX dev box is generating a couple of warnings since this commit:
>>> pg_dump.c:14548:45: warning: format specifies type 'long' but the
>>> argument has type 'long long' [-Wformat]
FWIW, I'm seeing the same thing on my OSX laptop. I think the explanation
is simple: both "long" and "long long" are 64 bits on this machine.
Our configure chooses the former to be "int64", but stdint.h is using
the latter. I'm not sure there's any good way to know what stdint.h
thinks is int64, but we can't do a half-baked integration job like this.
Either we revert 83ff1618b altogether, or we make int64/uint64 depend
on definitions from stdint.h rather than being chosen independently by
configure.
regards, tom lane
On 2015-03-30 15:38:16 -0400, Tom Lane wrote: > Michael Paquier <michael.paquier@gmail.com> writes: > > On Mon, Mar 30, 2015 at 7:48 PM, Andres Freund <andres@anarazel.de> wrote: > >> On 2015-03-30 14:01:25 +0900, Michael Paquier wrote: > >>> My OSX dev box is generating a couple of warnings since this commit: > >>> pg_dump.c:14548:45: warning: format specifies type 'long' but the > >>> argument has type 'long long' [-Wformat] > > FWIW, I'm seeing the same thing on my OSX laptop. I think the explanation > is simple: both "long" and "long long" are 64 bits on this machine. > Our configure chooses the former to be "int64", but stdint.h is using > the latter. I'm not sure there's any good way to know what stdint.h > thinks is int64, but we can't do a half-baked integration job like this. > Either we revert 83ff1618b altogether, or we make int64/uint64 depend > on definitions from stdint.h rather than being chosen independently by > configure. Obviously we need to fix this, but I'd actually say this isn't originally the fault of that change. The disparity in types already existed in a bunch of places (timestamp.c, pgbench.c), now it's more central, that's all. I'm too fried from the redeye back from pgconf nyc to do anything complicated, but it seems quite possible to define int64/uint64 based the stdint.h types if available. And generally a good idea too. I guess I'll try that tomorrow; unless Andrew beats me to it. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services
Hi, On 2015-03-30 21:50:09 +0200, Andres Freund wrote: > I'm too fried from the redeye back from pgconf nyc to do anything > complicated, but it seems quite possible to define int64/uint64 based > the stdint.h types if available. And generally a good idea too. I guess > I'll try that tomorrow; unless Andrew beats me to it. It's possible to do that, but it's not as trivial as I'd hoped. For one we'd need to include stdint.h in some places we don't today (postgres_ext.h), for another we'd need some uglyness to determine the correct printf modifier for int64_t (can't use PRId64 etc afaics). I'm tempted to just prefix our limits with PG_ and define them unconditionally, including appropriate casts to our types. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services
Andres Freund <andres@anarazel.de> writes:
> On 2015-03-30 21:50:09 +0200, Andres Freund wrote:
>> I'm too fried from the redeye back from pgconf nyc to do anything
>> complicated, but it seems quite possible to define int64/uint64 based
>> the stdint.h types if available. And generally a good idea too. I guess
>> I'll try that tomorrow; unless Andrew beats me to it.
> It's possible to do that, but it's not as trivial as I'd hoped. For one
> we'd need to include stdint.h in some places we don't today
> (postgres_ext.h), for another we'd need some uglyness to determine the
> correct printf modifier for int64_t (can't use PRId64 etc afaics).
Yeah, I thought the printf strings would be the sticking point :-(
> I'm tempted to just prefix our limits with PG_ and define them
> unconditionally, including appropriate casts to our types.
I don't have a better idea.
regards, tom lane
On 2015-03-31 12:10:48 -0400, Tom Lane wrote: > Andres Freund <andres@anarazel.de> writes: > > On 2015-03-30 21:50:09 +0200, Andres Freund wrote: > >> I'm too fried from the redeye back from pgconf nyc to do anything > >> complicated, but it seems quite possible to define int64/uint64 based > >> the stdint.h types if available. And generally a good idea too. I guess > >> I'll try that tomorrow; unless Andrew beats me to it. > > > It's possible to do that, but it's not as trivial as I'd hoped. For one > > we'd need to include stdint.h in some places we don't today > > (postgres_ext.h), for another we'd need some uglyness to determine the > > correct printf modifier for int64_t (can't use PRId64 etc afaics). > > Yeah, I thought the printf strings would be the sticking point :-( I hacked things till it worked, but it seems fragile. Using Werror for the format string test and adding 'l' to the tested combination works, but... At the point where it basically worked the required changes already amounted to ~150 lines changed (excluding configure). Making that robust is more than I'm willing to do right now. I do think it'd generally not be a bad thing to base our types on the standard types if available. > > I'm tempted to just prefix our limits with PG_ and define them > > unconditionally, including appropriate casts to our types. > > I don't have a better idea. Will push that. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services
On Tue, Mar 31, 2015 at 12:15 PM, Andres Freund <andres@anarazel.de> wrote: >> > I'm tempted to just prefix our limits with PG_ and define them >> > unconditionally, including appropriate casts to our types. >> >> I don't have a better idea. > > Will push that. I'd appreciate it if you could do this soon. I like to compile with -Werror, and this problem means I can't. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Re: [COMMITTERS] pgsql: Centralize definition of integer limits.
От
andres@anarazel.de (Andres Freund)
Дата:
On 2015-04-02 10:42:58 -0400, Robert Haas wrote: > On Tue, Mar 31, 2015 at 12:15 PM, Andres Freund <andres@anarazel.de> wrote: > >> > I'm tempted to just prefix our limits with PG_ and define them > >> > unconditionally, including appropriate casts to our types. > >> > >> I don't have a better idea. > > > > Will push that. > > I'd appreciate it if you could do this soon. I like to compile with > -Werror, and this problem means I can't. Done. Sorry for not doing this sooner, I'm more or less having holidays right now. Greetings, Andres Freund
On Thu, Apr 2, 2015 at 11:54 AM, Andres Freund <andres@anarazel.de> wrote: > On 2015-04-02 10:42:58 -0400, Robert Haas wrote: >> On Tue, Mar 31, 2015 at 12:15 PM, Andres Freund <andres@anarazel.de> wrote: >> >> > I'm tempted to just prefix our limits with PG_ and define them >> >> > unconditionally, including appropriate casts to our types. >> >> >> >> I don't have a better idea. >> > >> > Will push that. >> >> I'd appreciate it if you could do this soon. I like to compile with >> -Werror, and this problem means I can't. > > Done. Sorry for not doing this sooner, I'm more or less having holidays > right now. Thanks. Sorry to interrupt your vacation. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company