GHA PG16 tests failing on all warnings being treated as errors

[robe]

So I enabled PG16 on github actions and it's failing because of all sorts of warnings.

​https://github.com/postgis/postgis/actions/runs/3380700459/jobs/5613777652 Most notably in

gserialized_spgist_2d.c (use of error: passing argument 1 of 'DatumGetPointer' makes integer from pointer without a cast [-Werror=int-conversion] )

lwgeom_triggers.c:113:25: note: in expansion of macro 'DirectFunctionCall1'
  113 |   out = PointerGetDatum(DirectFunctionCall1(LWGEOM_addBBOX, in));
      |                         ^~~~~~~~~~~~~~~~~~~
In file included from lwgeom_triggers.c:26:
/usr/local/pgsql/include/server/postgres.h:670:29: note: expected 'const void *' but argument is of type 'Datum' {aka 'long unsigned int'}
  670 | PointerGetDatum(const void *X)

gserialized_spgist_3d.c:433:9: note: shadowed declaration is here
  433 |  BOX3D *box = DatumGetBox3DP(in->datums[0]);
      |         ^~~
In file included from ../libpgcommon/lwgeom_pg.h:22,
                 from gserialized_gist_2d.c:51:
/usr/local/pgsql/include/server/utils/geo_decls.h:239:25: note: expected 'const BOX *' but argument is of type 'BOX2DF *'
  239 | BoxPGetDatum(const BOX *X)
      |              ~~~~~~~~~~~^
gserialized_spgist_3d.c:484:10: error: declaration of 'box' shadows a previous local [-Werror=shadow=compatible-local]

I didn't think PG16 changed that recently, so these might exist in earlier. Looks like only PG14 GHA is testing in tests mode which treats all warnings as errors, and that one is running fine.

[Regina Obe <lr@…>]

In 2405b53/git:

Change PG16 to not treat warnings as errors. References #5278

[strk]

GHA is all green now, I guess this ticket can be closed

[robe]

No GHA is green because I changed the tests from usan_clang/tests to coverage.

coverage GHA script doesn't treat warnings as errors. But we do have a ton of warnings now which weren't present with PG15.

A lot of stuff they seem to be doing in the PG16 cycle is replacing their home grown functions with standard C ones. So I think that is what is causing these warnings. I haven't investigated in detail yet.

[strk]

The reported shadowed BOX in gserialized_spgist_3d.c:433 seems to be a real error to me as the box received as parameter by the picksplit 3d function is ONLY used for the SRID and nothing else. \cc @pramsey

[Regina Obe <lr@…>]

In 9401093/git:

FIX GHA 16 latest warnings
Closes #5278 for PostGIS 3.4.0

• Turn back on warnings notices on latest (switch back tests mode)
• Fix shadow warnings on gserialized gist c files
• Fix shawow warning on mvt
• Fix double-free warning on address_standardizer
• Fix strncopy length depends on bounds of source instead of dest

[strk]

The change in postgis/gserialized_spgist_3d.c looks dangerous. It would be good if Paul could take a look. The BOX3D "box" variable is first initialized with in→datums[0]and then again in a loop over in→nTuples. Should it not be initialized upfront at all ? Should it be declared constant ? I would do both things but I'm not sure how easy it is to test these things.

[strk]

We seem to assume there's always at least one tuple and we use that to fetch the SRID, I'll make that more explicit

[Sandro Santilli <strk@…>]

In 3874270/git:

const-correct getOctant

References #5278

[Paul Ramsey <pramsey@…>]

In 1985836/git:

Change local var names to avoid shadows, references #5278