Use of PRId64 with PostgreSQL functions

Paragon Corporation wrote:

I've been currently suffering an issue in mingw64 with changes made in
PostGIS topology code base. Sadly it only happens to me (compiling in
mingw64)

Issue is when code like this was introduced

appendStringInfo(str, "%s%" PRId64, sep, edge->start_node);

I think you could use INT64_FORMAT for this.

--
�lvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

"Paragon Corporation" <lr@pcorp.us> writes:

I've been currently suffering an issue in mingw64 with changes made in
PostGIS topology code base. Sadly it only happens to me (compiling in
mingw64)

Issue is when code like this was introduced
appendStringInfo(str, "%s%" PRId64, sep, edge->start_node);

Which uses a PostgreSQL function appendStringInfo - as noted here:
https://trac.osgeo.org/postgis/ticket/3206

Huh. Apparently, whichever Windows compiler you're using defines
PRId64 as "d", which surely seems pretty broken.

I noticed a thread in March on pgsql-hackers that discussed int limits
/messages/by-id/20150331141423.GK4878@alap3.anarazel.de
in PostgreSQL and Andres Freund made this comment which stuck in my head:
" for another we'd need some uglyness to determine the
correct printf modifier for int64_t (can't use PRId64 etc afaics)."
What exactly did he mean by can't use PRId64?

That was about the fact that we don't know whether PRId64 is defined
as "ld" or "lld" on machines where either of those could legitimately
mean a 64-bit int. (And while either would work at runtime in that
case, we'd get compiler warnings we don't want if we chose the wrong
one.) That problem doesn't apply to Windows AFAIK.

However, some googling suggests that PRId64 isn't defined on all Windows
build environments, and your report says that there's at least one such
environment where it is defined, but wrongly :-(. So that's yet another
good reason to stay away from PRId64. You'll notice that it appears
nowhere in the Postgres sources.

You did not say how you're declaring the variable that's being printed
here, but if it's based on the int64 type declared by c.h, you should
use the INT64_FORMAT or INT64_MODIFIER strings declared by c.h/pg_config.h.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Alvaro and Tom,

A big thank you. That seems to have done the trick. My compiler warnings
went away and no more syntax errors in the regress tests. I still have some
regress failures I got to hunt down, but those could be caused by places in
the code I did not replace PRId64 calls or something else.

Huh. Apparently, whichever Windows compiler you're using defines
PRId64 as "d", which surely seems pretty broken.

I think it was only failing on the postgresql function calls.
The simple test Sandro Santilli had me run

--- testint.c ---
#include <stdint.h>
#include <inttypes.h>
#include <stdio.h>
int main()
{
  int64_t x = 1;
  printf("%" PRId64, x);
  return 0;
}

Printed: 1

And this: gcc -Wall -E testint.c | grep '\(printf.*x)\| int64_t;\)'
Returned:
__extension__ typedef long long int64_t;
printf("%" "I64d", x);

You did not say how you're declaring the variable that's being printed

here, but if it's based on the int64 type declared by c.h, you should use
the INT64_FORMAT or INT64_MODIFIER strings declared by c.h/pg_config.h.

The types are defined as:

/* INT64 */
typedef int64_t LWT_INT64;
/** Identifier of topology element */
typedef LWT_INT64 LWT_ELEMID;

in this file.
https://trac.osgeo.org/postgis/browser/trunk/liblwgeom/liblwgeom_topo.h

Anyway thanks again. Very much appreciated.

Regina

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

"Paragon Corporation" <lr@pcorp.us> writes:

Huh. Apparently, whichever Windows compiler you're using defines
PRId64 as "d", which surely seems pretty broken.

I think it was only failing on the postgresql function calls.

And this: gcc -Wall -E testint.c | grep '\(printf.*x)\| int64_t;\)'
Returned:
__extension__ typedef long long int64_t;
printf("%" "I64d", x);

Oooh ... so the issue is the nonstandard "I64" length modifier.
Now I understand. That probably works fine with Microsoft's printf,
but

(1) gcc does not understand it, which is why you're getting the compile
warnings with the erroneous claim that the format is just "d", and

(2) our implementation of snprintf doesn't understand it either, which
presumably is causing weird runtime behavior for you when you try to
use this with StringInfo functions.

Adherence to standards was never MS' strong point was it.

Anyway, it would be advisable to switch your declarations to use PG's
int64 if you are going to use INT64_FORMAT. If you use that macro with a
platform-provided int64_t declaration then you are going to get compile
warnings on some platforms; that is exactly the point that Andres was
making in the previous discussion.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers