Обсуждение: Windows 64 bit warnings
While looking into setting up some libraries to use for 64 bit Windows
builds, I took a quick look at the output from the 64 bit postgres
builds currently running. They're actually quite clean, a heck of a lot
cleaner than several other packages I have been looking at, quite a good
testament to the cleanliness of our code. I was looking specifically for
instances of warnings like "warning: cast to pointer from integer of
different size" or the other way around, and found just two.
One is at src/interfaces/ecpg/ecpglib/sqlda.c:231, which is this line:
sqlda->sqlvar[i].sqlformat = (char *) (long) PQfformat(res, i);
I'm not clear about the purpose of this anyway. It doesn't seem to be
used anywhere, and the comment on the field says it for future use. If
we're going to do it, I think it should be cast to a long long on Win64,
since a char * is 8 bytes there, while a long is only 4. But if we
really want to store the result from PQfformat() in it, why is it a char
* at all?
The other, slightly more serious case, is at
src/test/regress/pg_regress.c:2280, which is this code:
printf(_("running on port %d with pid %lu\n"), port, (unsigned long) postmaster_pid);
Here the postmaster_pid is in fact a HANDLE which is 8 bytes, and so it
should probably be cast to an unsigned long long and rendered with the
format %llu in Win64.
cheers
andrew
Excerpts from Andrew Dunstan's message of sáb abr 16 21:46:44 -0300 2011:
> The other, slightly more serious case, is at
> src/test/regress/pg_regress.c:2280, which is this code:
>
> printf(_("running on port %d with pid %lu\n"),
> port, (unsigned long) postmaster_pid);
>
> Here the postmaster_pid is in fact a HANDLE which is 8 bytes, and so it
> should probably be cast to an unsigned long long and rendered with the
> format %llu in Win64.
Is this "uint64" and UINT64_FORMAT?
--
Álvaro Herrera <alvherre@commandprompt.com>
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support
Alvaro Herrera <alvherre@commandprompt.com> writes:
> Excerpts from Andrew Dunstan's message of sáb abr 16 21:46:44 -0300 2011:
>> The other, slightly more serious case, is at
>> src/test/regress/pg_regress.c:2280, which is this code:
>>
>> printf(_("running on port %d with pid %lu\n"),
>> port, (unsigned long) postmaster_pid);
>>
>> Here the postmaster_pid is in fact a HANDLE which is 8 bytes, and so it
>> should probably be cast to an unsigned long long and rendered with the
>> format %llu in Win64.
> Is this "uint64" and UINT64_FORMAT?
Considering that this is a purely informational printout, I don't see
any reason to give a damn about the possibility of high-order bits in
the HANDLE being dropped. And it's not an especially good idea to stick
UINT64_FORMAT into a translatable string, because of the platform
dependency that creates.
I think all we need here is a way to shut up the overly-anal-retentive
warning. I would have expected that explicit cast to be enough,
actually, but apparently it's not. Ideas?
regards, tom lane
On Mon, Apr 18, 2011 at 19:00, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Alvaro Herrera <alvherre@commandprompt.com> writes:
>> Excerpts from Andrew Dunstan's message of sáb abr 16 21:46:44 -0300 2011:
>>> The other, slightly more serious case, is at
>>> src/test/regress/pg_regress.c:2280, which is this code:
>>>
>>> printf(_("running on port %d with pid %lu\n"),
>>> port, (unsigned long) postmaster_pid);
>>>
>>> Here the postmaster_pid is in fact a HANDLE which is 8 bytes, and so it
>>> should probably be cast to an unsigned long long and rendered with the
>>> format %llu in Win64.
>
>> Is this "uint64" and UINT64_FORMAT?
>
> Considering that this is a purely informational printout, I don't see
> any reason to give a damn about the possibility of high-order bits in
> the HANDLE being dropped. And it's not an especially good idea to stick
> UINT64_FORMAT into a translatable string, because of the platform
> dependency that creates.
IIRC, even while HANDLE is a 64-bit value on Win64, only the lower
32-bits are actually used. Took me a while to find a ref, but this is
one I found: http://msdn.microsoft.com/en-us/library/aa384203(v=vs.85).aspx
> I think all we need here is a way to shut up the overly-anal-retentive
> warning. I would have expected that explicit cast to be enough,
> actually, but apparently it's not. Ideas?
Not sure about that one. Certainly seems like that case should be
enough - it always was enough to silence similar warnings on MSVC in
the past...
--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/
On 04/19/2011 04:08 AM, Magnus Hagander wrote: > IIRC, even while HANDLE is a 64-bit value on Win64, only the lower > 32-bits are actually used. Took me a while to find a ref, but this is > one I found: http://msdn.microsoft.com/en-us/library/aa384203(v=vs.85).aspx Good. >> I think all we need here is a way to shut up the overly-anal-retentive >> warning. I would have expected that explicit cast to be enough, >> actually, but apparently it's not. Ideas? > Not sure about that one. Certainly seems like that case should be > enough - it always was enough to silence similar warnings on MSVC in > the past... > If we cast the HANDLE to a long long first and then truncate it the compiler is silent, it only complains if that's done in one operation. So maybe something like: #ifdef WIN64 #define ULONGPID(x) (unsigned long) (unsigned long long) (x) #else #define ULONGPID(x) (unsigned long)(x) #endif ... printf(_("running on port %d with pid %lu\n"), port, ULONGPID(postmaster_pid)); It's a bit ugly, but we've done worse. cheers andrew
On Sat, Apr 16, 2011 at 08:46:44PM -0400, Andrew Dunstan wrote: > One is at src/interfaces/ecpg/ecpglib/sqlda.c:231, which is this line: > > sqlda->sqlvar[i].sqlformat = (char *) (long) PQfformat(res, i); > > I'm not clear about the purpose of this anyway. It doesn't seem to > be used anywhere, and the comment on the field says it for future By used you mean inside the postgres right? If so this is not surprising, sqlda is used to give data to the program using the library. > use. If we're going to do it, I think it should be cast to a long > long on Win64, since a char * is 8 bytes there, while a long is only > 4. But if we really want to store the result from PQfformat() in it, > why is it a char * at all? Zoltan, did you see this? Given that you wrote the code it might be good if you could comment on this. 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 Jabber: michael.meskes at googlemail dot com VfL Borussia! Força Barça! Go SF 49ers! Use Debian GNU/Linux, PostgreSQL
Andrew Dunstan <andrew@dunslane.net> writes:
> If we cast the HANDLE to a long long first and then truncate it the
> compiler is silent, it only complains if that's done in one operation.
> So maybe something like:
> #ifdef WIN64
> #define ULONGPID(x) (unsigned long) (unsigned long long) (x)
> #else
> #define ULONGPID(x) (unsigned long) (x)
> #endif
... with a comment, please. Perhaps
#ifdef WIN64 /* need a series of two casts to convert HANDLE without compiler warning */ #define ULONGPID(x)
(unsignedlong) (unsigned long long) (x) #else #define ULONGPID(x) (unsigned long) (x) #endif
regards, tom lane
> One is at src/interfaces/ecpg/ecpglib/sqlda.c:231, which is this line: > > sqlda->sqlvar[i].sqlformat = (char *) (long) PQfformat(res, i); > > I'm not clear about the purpose of this anyway. It doesn't seem to After not hearing from the author I just commented out that line. I cannot find any explanation what should be stored in sqlformat. > be used anywhere, and the comment on the field says it for future Gosh, I didn't know that our own source says it's reserved for future use. I guess that makes removing the statement even more of an option. 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 Jabber: michael.meskes at googlemail dot com VfL Borussia! Força Barça! Go SF 49ers! Use Debian GNU/Linux, PostgreSQL