Windows 64 bit warnings

Started by Andrew Dunstanover 14 years ago8 messages
#1Andrew Dunstan
andrew@dunslane.net

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

#2Alvaro Herrera
alvherre@commandprompt.com
In reply to: Andrew Dunstan (#1)
Re: Windows 64 bit warnings

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

#3Tom Lane
tgl@sss.pgh.pa.us
In reply to: Alvaro Herrera (#2)
Re: Windows 64 bit warnings

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

#4Magnus Hagander
magnus@hagander.net
In reply to: Tom Lane (#3)
Re: Windows 64 bit warnings

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/

#5Andrew Dunstan
andrew@dunslane.net
In reply to: Magnus Hagander (#4)
Re: Windows 64 bit warnings

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

#6Michael Meskes
meskes@postgresql.org
In reply to: Andrew Dunstan (#1)
Re: Windows 64 bit warnings

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

#7Tom Lane
tgl@sss.pgh.pa.us
In reply to: Andrew Dunstan (#5)
Re: Windows 64 bit warnings

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) (unsigned long) (unsigned long long) (x)
#else
#define ULONGPID(x) (unsigned long) (x)
#endif

regards, tom lane

#8Michael Meskes
meskes@postgresql.org
In reply to: Andrew Dunstan (#1)
Re: Windows 64 bit warnings

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