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

Search Discussions

  • Alvaro Herrera at Apr 18, 2011 at 4:51 pm

    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
  • Tom Lane at Apr 18, 2011 at 10:01 pm

    Alvaro Herrera 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
  • Magnus Hagander at Apr 19, 2011 at 8:09 am

    On Mon, Apr 18, 2011 at 19:00, Tom Lane 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...
  • Andrew Dunstan at Apr 19, 2011 at 11:45 am

    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
  • Tom Lane at Apr 19, 2011 at 3:12 pm

    Andrew Dunstan 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
  • Michael Meskes at Apr 19, 2011 at 1:44 pm

    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
  • Michael Meskes at Apr 25, 2011 at 11:48 am

    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

Related Discussions

Discussion Navigation
viewthread | post
Discussion Overview
grouppgsql-hackers @
categoriespostgresql
postedApr 17, '11 at 12:47a
activeApr 25, '11 at 11:48a
posts8
users5
websitepostgresql.org...
irc#postgresql

People

Translate

site design / logo © 2021 Grokbase