[Postgres-xl-developers] treating coordinator_lxid GUC as a string with check/assign/show functions

Tomas Vondra tomas.vondra at 2ndquadrant.com
Tue Jun 27 16:17:34 PDT 2017


Hi,

One of the remaining warnings produced by gcc (and other compilers) is 
this one, related to the coordinator_lxid UC definition:

     guc.c:3085:3: warning: pointer targets in initialization differ in
                   signedness [-Wpointer-sign]
        &MyCoordLxid,
        ^
     guc.c:3085:3: note: (near initialization for
                         ‘ConfigureNamesInt[109].variable’)


The issue here is that the GUC is defined as ConfigureNamesInt and so is 
treated as signed int, while MyCoordLxid is uint32.

We don't have any other uint32 GUCs, so there's no config_uint struct at 
the moment. Adding one would be one way to fix this, of course. The 
attached patch however fixes that simply by treating the GUC as a text, 
and adds hooks to parse/assign/show the value. But perhaps adding 
config_uint would be cleaner?

What however worries me a bit is that a few other places dealing with 
the value assume it's a signed integer. Consider for example this bit in 
execRemote.c

     sprintf(lxid, "%d", MyProc->lxid);

which converts the uint32 value to a string as if it was a signed 
integer. I assume this relies on the "right thing" to happen during the 
signed/unsigned conversion, and maybe it's fine. But it seems confusing 
at best, so we should probably tweak it to use '%u' instead.

Opinions?


regards

-- 
Tomas Vondra                  http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
-------------- next part --------------
A non-text attachment was scrubbed...
Name: coordinator-lxid-guc.patch
Type: text/x-patch
Size: 3206 bytes
Desc: not available
URL: <http://lists.postgres-xl.org/private.cgi/postgres-xl-developers-postgres-xl.org/attachments/20170628/bf734ea3/attachment.bin>


More information about the Postgres-xl-developers mailing list