Opened 9 years ago

Closed 9 years ago

#3243 closed defect (fixed)

piles 'o clang warnings in topology

Reported by: pramsey Owned by: strk
Priority: medium Milestone: PostGIS 2.2.0
Component: topology Version: master
Keywords: Cc:

Description

 warning: format specifies type 'long' but the argument has type 'LWT_ELEMID' (aka 'long long') [-Wformat]
    topo->name, topo->id, rem_edge );

times 200.

Change History (17)

comment:1 by strk, 9 years ago

On what system ? Is this in liblwgeom/lwgeom_topo.c or topology/postgis_topology.c ? The latter uses a PostgreSQL provided macro, the former uses our own, which is in stdint.h by default AND specially-handled for WIN32 (LWTFMT_ELEMID).

comment:2 by pramsey, 9 years ago

postgis_topology.c:3277:15: warning: format specifies type 'long' but the argument has type 'LWT_ELEMID' (aka 'long long') [-Wformat]
              state->elems[state->curr]) >= 32 )
              ^~~~~~~~~~~~~~~~~~~~~~~~~

comment:3 by strk, 9 years ago

Line 3277 of postgis_topology.c reads:

snprintf( values[1], 32, INT64_FORMAT, state->elems[state->curr] )

Your error message says that INT64_FORMAT expands to 'long', while LWT_ELEMID (int64_t from stdint.h) is 'long long'. What system are you on ? Is it a 32bit or 64bit ? Is it PostgreSQL's INT64_FORMAT being wrong or stdint.h's int64_t being wrong ?

comment:4 by strk, 9 years ago

Do you get any such warning under liblwgeom ? As we might switch to using our own macro also under postgis, if PostgreSQL's isn't good enough (would be also more consistent)

comment:5 by robe, 9 years ago

Sorry guys just noticed I'm getting these warnings too when compiling 32-bit. Though it's odd I don't see the same warnings on winnie and we should be using the same mingw64 chain. Though it might be a difference of postgres. Her 32-bit is running 9.4.1 and i=mine is 9.4.2. I'm going to verify.

comment:6 by robe, 9 years ago

I was mistaken. I see these on winnie's 32 and64-bit builds too and my warnings are different than pramsey's.

---- Making all in topology
make[1]: Entering directory `/projects/postgis/branches/2.2/topology'
gcc -Wall -Wmissing-prototypes -Wpointer-arith -Wdeclaration-after-statement -Wendif-labels -Wmissing-format-attribute -Wformat-security -fno-strict-aliasing -fwrapv -fexcess-precision=standard -g -O2  -I../liblwgeom -I../libpgcommon  -I/projects/geos/rel-3.5.0w32gcc481/include -I/projects/proj/rel-4.9.1w32gcc481/include -I/projects/libxml/rel-libxml2-2.7.8w32gcc481/include/libxml2  -I/projects/json-c/rel-0.12w32gcc481/include  -I/projects/postgresql/r
postgis_topology.c:1:0: warning: -fPIC ignored for target (all code is position independent) [enabled by default]
 /**********************************************************************
 ^
postgis_topology.c: In function 'cb_checkTopoGeomRemEdge':
postgis_topology.c:2106:13: warning: unknown conversion type character 'l' in format [-Wformat=]
             col_name, rem_edge);
             ^
postgis_topology.c:2106:13: warning: too many arguments for format [-Wformat-extra-args]
postgis_topology.c:2161:15: warning: unknown conversion type character 'l' in format [-Wformat=]
               col_name, face_right, face_left);
               ^
postgis_topology.c:2161:15: warning: unknown conversion type character 'l' in format [-Wformat=]
postgis_topology.c:2161:15: warning: too many arguments for format [-Wformat-extra-args]
postgis_topology.c: In function 'cb_checkTopoGeomRemNode':
postgis_topology.c:2222:13: warning: unknown conversion type character 'l' in format [-Wformat=]
             col_name, edge1, edge2);
             ^
postgis_topology.c:2222:13: warning: unknown conversion type character 'l' in format [-Wformat=]
postgis_topology.c:2222:13: warning: too many arguments for format [-Wformat-extra-args]
gcc -Wall -Wmissing-prototypes -Wpointer-arith -Wdeclaration-after-statement -Wendif-labels -Wmissing-format-attribute -Wformat-security -fno-strict-aliasing -fwrapv -fexcess-precision=standard -g -O2   -shared -static-libgcc -o postgis_topology-2.2.dll  postgis_topology.o -Lc:/MING32~2/projects/POSTGR~1/rel/PG9~1.4W3/lib -Wl,--allow-multiple-definition  -Wl,--as-needed   ../libpgcommon/libpgcommon.a ../liblwgeom/.libs/liblwgeom.a  -L/projects/geos/rel
/mingw/bin/cpp -traditional-cpp -w -P topology.sql.in | grep -v '^#' | \
/bin/perl -lpe "s'MODULE_PATHNAME'\$libdir/postgis_topology-2.2'g" > topology.sql
/mingw/bin/cpp -traditional-cpp -w -P topology_drop_before.sql.in | grep -v '^#' | \
/bin/perl -lpe "s'MODULE_PATHNAME'\$libdir/postgis_topology-2.2'g" > topology_drop_before.sql
/bin/perl -0777 -ne 's/^(CREATE|ALTER) (CAST|OPERATOR|TYPE|TABLE|SCHEMA|DOMAIN|TRIGGER).*?;//msg;print;' topology.sql > topology_upgrade.sql.in
/mingw/bin/cpp -traditional-cpp -w -P topology_drop_after.sql.in | grep -v '^#' | \
/bin/perl -lpe "s'MODULE_PATHNAME'\$libdir/postgis_topology-2.2'g" > topology_drop_after.sql
cat topology_drop_before.sql topology_upgrade.sql.in topology_drop_after.sql > topology_upgrade.sql

comment:7 by strk, 9 years ago

Could you please check if this fixes your warnings ? https://github.com/postgis/postgis/pull/62

comment:8 by strk, 9 years ago

Please try r14020

comment:9 by robe, 9 years ago

nope still getting:

 ^
postgis_topology.c: In function 'cb_checkTopoGeomRemEdge':
postgis_topology.c:2112:13: warning: unknown conversion type character 'l' in format [-Wformat=]
             col_name, rem_edge);
             ^
postgis_topology.c:2112:13: warning: too many arguments for format [-Wformat-extra-args]
postgis_topology.c:2168:15: warning: unknown conversion type character 'l' in format [-Wformat=]
               col_name, face_right, face_left);
               ^
postgis_topology.c:2168:15: warning: unknown conversion type character 'l' in format [-Wformat=]
postgis_topology.c:2168:15: warning: too many arguments for format [-Wformat-extra-args]
postgis_topology.c: In function 'cb_checkTopoGeomRemNode':
postgis_topology.c:2229:13: warning: unknown conversion type character 'l' in format [-Wformat=]
             col_name, edge1, edge2);

Everything still passes though so lets see if it fixes pramsey's issues. I think we have different issues.

comment:10 by pramsey, 9 years ago

I just ran a build and it was warning free. OSX is clean.

comment:11 by strk, 9 years ago

Regina which revision and which operating system ?

comment:12 by robe, 9 years ago

revision r14020 and windows 64-bit PostGIS compiled with mingw64 gcc 4.8.3. I think I have the same issue on my 32-bit run, but have to double check that.

Yours may never have been giving a warning at that spot.

comment:13 by strk, 9 years ago

robe, we're defining LWTFMT_ELEMID just for WIN32, see line 42, now your compiler says it does not know the conversion type character 'l' ? What's the correct format then ? Or is "WIN32" not defined there, being a WIN64 ? How to check if it's a windows machine despite word size ?

comment:14 by robe, 9 years ago

It works fine in other places -- it's only the cb_checkTopoGeomRemEdge where it's giving the warning. WIN32 catches both win32 and win64. I should add I see the warning on both the 32-bit and 64-bit runs.

Last edited 9 years ago by robe (previous) (diff)

comment:15 by robe, 9 years ago

I was reading about this some more. Not sure why the warning is only happening in cb_checkTopoGeomRemEdge, but things I've read say that mingw64 puts out this mostly harmless warning because some older msvcrt dlls don't support l. So since at runtime it relies on msvcrt, it could end up using a runtime of msvcrt that doesn't support.

http://comments.gmane.org/gmane.comp.gnu.mingw.w64.general/4670

One suggestion use define USE_MINGW_ANSI_STDIO macro before

including stdio.h. as described here: http://permalink.gmane.org/gmane.comp.gnu.mingw.w64.general/4674

I'm not sure it's worth fixing, as I suspect whatever msvcrt doesn't support this might be too old for me to care about.

I'm also a bit puzzed since the warning is coming on a postgresql append string call which I thought only uses ANSI int implementations (which is why we can't use the PRI thing in the first place)

comment:16 by robe, 9 years ago

Okay I was mistaken -- it is failing in the cberror call not the appendinfostring calls.

So that explains why I'm only getting the warning in those since in other places you have %d.

Like I said though the IF define is fine. The issue is your cberror would rely on msvcrt implementation (the Postgres one needs ANSI implementation) so we'd need two different formats to handle -- not sure its worth the trouble.

Version 0, edited 9 years ago by robe (next)

comment:17 by robe, 9 years ago

Resolution: fixed
Status: newclosed

Talked with strk - he's okay if I want to close this. Like I said I figure this only affects Windows XP users which isn't really a supported platform for windows anymore. I checked the ether and it seems lld is supported on windows 7+ and windows 2012 (I'm guessing windows 2008 as well since I think windows 2008 / windows 7 are based on same msvcrt).

Note: See TracTickets for help on using tickets.