Thread: Fix shadow warnings in logical replication code

Fix shadow warnings in logical replication code

From
Peter Smith
Date:
The -Wshadow compiler option reported 3 shadow warnings within the
logical replication files. (These are all in old code)

PSA a patch to address those.

======

logicalfuncs.c:184:13: warning: declaration of ‘name’ shadows a
previous local [-Wshadow]
    char    *name = TextDatumGetCString(datum_opts[i]);
             ^
logicalfuncs.c:105:8: warning: shadowed declaration is here [-Wshadow]
  Name  name;
        ^

~~~

reorderbuffer.c:4843:10: warning: declaration of ‘isnull’ shadows a
previous local [-Wshadow]
    bool  isnull;
          ^
reorderbuffer.c:4734:11: warning: shadowed declaration is here [-Wshadow]
  bool    *isnull;
           ^

~~~

walsender.c:3543:14: warning: declaration of ‘sentPtr’ shadows a
global declaration [-Wshadow]
   XLogRecPtr sentPtr;
              ^
walsender.c:155:19: warning: shadowed declaration is here [-Wshadow]
 static XLogRecPtr sentPtr = InvalidXLogRecPtr;
                   ^

------
Kind Regards,
Peter Smith.
Fujitsu Australia

Attachment

Re: Fix shadow warnings in logical replication code

From
Michael Paquier
Date:
On Wed, Aug 30, 2023 at 09:16:38AM +1000, Peter Smith wrote:
> logicalfuncs.c:184:13: warning: declaration of ‘name’ shadows a
> previous local [-Wshadow]
>     char    *name = TextDatumGetCString(datum_opts[i]);
>              ^
> logicalfuncs.c:105:8: warning: shadowed declaration is here [-Wshadow]
>   Name  name;

A bit confusing here, particularly as the name is reused with
ReplicationSlotAcquire() at the end of
pg_logical_slot_get_changes_guts() once again.

> reorderbuffer.c:4843:10: warning: declaration of ‘isnull’ shadows a
> previous local [-Wshadow]
>     bool  isnull;
>           ^
> reorderbuffer.c:4734:11: warning: shadowed declaration is here [-Wshadow]
>   bool    *isnull;
>            ^

Agreed as well about this one.

> walsender.c:3543:14: warning: declaration of ‘sentPtr’ shadows a
> global declaration [-Wshadow]
>    XLogRecPtr sentPtr;
>               ^
> walsender.c:155:19: warning: shadowed declaration is here [-Wshadow]
>  static XLogRecPtr sentPtr = InvalidXLogRecPtr;
>                    ^

This one looks pretty serious to me, particularly as the static
sentPtr is used quite a bit.  It is fortunate that the impact is
limited to the WAL sender stat function.

Fixing all these seems like a good thing in the long term, so OK for
me.  Like all the fixes similar to this one, I don't see a need for a
backpatch based on their locality, even if sentPtr makes me a bit
nervous to keep even in stable branches.

There is much more going on with -Wshadow, but let's do things
incrementally, case by case.
--
Michael

Attachment

Re: Fix shadow warnings in logical replication code

From
Richard Guo
Date:

On Wed, Aug 30, 2023 at 8:49 AM Michael Paquier <michael@paquier.xyz> wrote:
There is much more going on with -Wshadow, but let's do things
incrementally, case by case.

Yeah, IIRC the source tree currently is able to be built without any
shadow-related warnings with -Wshadow=compatible-local.  But with
-Wshadow or -Wshadow=local, you can still see a lot of warnings.

Thanks
Richard

Re: Fix shadow warnings in logical replication code

From
Michael Paquier
Date:
On Wed, Aug 30, 2023 at 09:50:17AM +0800, Richard Guo wrote:
> Yeah, IIRC the source tree currently is able to be built without any
> shadow-related warnings with -Wshadow=compatible-local.  But with
> -Wshadow or -Wshadow=local, you can still see a lot of warnings.

Yep.  I've addressed on HEAD the ones proposed on this thread.
--
Michael

Attachment

Re: Fix shadow warnings in logical replication code

From
Peter Smith
Date:
On Thu, Aug 31, 2023 at 9:39 AM Michael Paquier <michael@paquier.xyz> wrote:
>
> On Wed, Aug 30, 2023 at 09:50:17AM +0800, Richard Guo wrote:
> > Yeah, IIRC the source tree currently is able to be built without any
> > shadow-related warnings with -Wshadow=compatible-local.  But with
> > -Wshadow or -Wshadow=local, you can still see a lot of warnings.
>
> Yep.  I've addressed on HEAD the ones proposed on this thread.
> --

Thanks for pushing!

3 gone, ~150 remaining :)

------
Kind Regards,
Peter Smith.
Fujitsu Australia

~

[postgres@CentOS7-x64 oss_postgres_misc]$ cat make.txt | grep
'warning: declaration'
controldata_utils.c:52:29: warning: declaration of ‘DataDir’ shadows a
global declaration [-Wshadow]
controldata_utils.c:145:32: warning: declaration of ‘DataDir’ shadows
a global declaration [-Wshadow]
brin.c:485:16: warning: declaration of ‘tmp’ shadows a previous local [-Wshadow]
gistbuild.c:1225:23: warning: declaration of ‘splitinfo’ shadows a
previous local [-Wshadow]
xlogreader.c:108:24: warning: declaration of ‘wal_segment_size’
shadows a global declaration [-Wshadow]
xlogrecovery.c:1170:13: warning: declaration of ‘backupEndRequired’
shadows a global declaration [-Wshadow]
xlogrecovery.c:1832:33: warning: declaration of ‘xlogreader’ shadows a
global declaration [-Wshadow]
xlogrecovery.c:3047:28: warning: declaration of ‘xlogprefetcher’
shadows a global declaration [-Wshadow]
xlogrecovery.c:3051:19: warning: declaration of ‘xlogreader’ shadows a
global declaration [-Wshadow]
xlogrecovery.c:3214:31: warning: declaration of ‘xlogreader’ shadows a
global declaration [-Wshadow]
xlogrecovery.c:3965:38: warning: declaration of ‘xlogprefetcher’
shadows a global declaration [-Wshadow]
objectaddress.c:2173:12: warning: declaration of ‘nulls’ shadows a
previous local [-Wshadow]
objectaddress.c:2190:12: warning: declaration of ‘nulls’ shadows a
previous local [-Wshadow]
objectaddress.c:2227:12: warning: declaration of ‘nulls’ shadows a
previous local [-Wshadow]
pg_constraint.c:811:22: warning: declaration of ‘cooked’ shadows a
parameter [-Wshadow]
parse_target.c:1647:13: warning: declaration of ‘levelsup’ shadows a
parameter [-Wshadow]
extension.c:1079:13: warning: declaration of ‘schemaName’ shadows a
parameter [-Wshadow]
schemacmds.c:208:12: warning: declaration of ‘stmt’ shadows a
parameter [-Wshadow]
statscmds.c:291:16: warning: declaration of ‘attnums’ shadows a
previous local [-Wshadow]
tablecmds.c:14034:20: warning: declaration of ‘cmd’ shadows a
parameter [-Wshadow]
tablecmds.c:14107:20: warning: declaration of ‘cmd’ shadows a
parameter [-Wshadow]
trigger.c:1170:13: warning: declaration of ‘qual’ shadows a previous
local [-Wshadow]
execExprInterp.c:407:27: warning: declaration of ‘dispatch_table’
shadows a global declaration [-Wshadow]
nodeAgg.c:3978:20: warning: declaration of ‘phase’ shadows a previous
local [-Wshadow]
nodeValuesscan.c:145:16: warning: declaration of ‘estate’ shadows a
previous local [-Wshadow]
be-secure-common.c:110:44: warning: declaration of ‘ssl_key_file’
shadows a global declaration [-Wshadow]
main.c:217:27: warning: declaration of ‘progname’ shadows a global
declaration [-Wshadow]
main.c:327:18: warning: declaration of ‘progname’ shadows a global
declaration [-Wshadow]
main.c:386:24: warning: declaration of ‘progname’ shadows a global
declaration [-Wshadow]
equivclass.c:727:16: warning: declaration of ‘rel’ shadows a parameter
[-Wshadow]
createplan.c:1245:12: warning: declaration of ‘plan’ shadows a
previous local [-Wshadow]
createplan.c:2560:12: warning: declaration of ‘plan’ shadows a
previous local [-Wshadow]
partdesc.c:218:16: warning: declaration of ‘key’ shadows a previous
local [-Wshadow]
logicalfuncs.c:184:13: warning: declaration of ‘name’ shadows a
previous local [-Wshadow]
reorderbuffer.c:4843:10: warning: declaration of ‘isnull’ shadows a
previous local [-Wshadow]
walsender.c:3543:14: warning: declaration of ‘sentPtr’ shadows a
global declaration [-Wshadow]
dependencies.c:377:23: warning: declaration of ‘DependencyGenerator’
shadows a global declaration [-Wshadow]
dependencies.c:1194:14: warning: declaration of ‘expr’ shadows a
parameter [-Wshadow]
dependencies.c:1228:22: warning: declaration of ‘expr’ shadows a
parameter [-Wshadow]
extended_stats.c:1047:10: warning: declaration of ‘isnull’ shadows a
previous local [-Wshadow]
date.c:506:9: warning: declaration of ‘days’ shadows a global
declaration [-Wshadow]
date.c:530:9: warning: declaration of ‘days’ shadows a global
declaration [-Wshadow]
date.c:2015:9: warning: declaration of ‘days’ shadows a global
declaration [-Wshadow]
datetime.c:637:8: warning: declaration of ‘days’ shadows a global
declaration [-Wshadow]
formatting.c:3181:25: warning: declaration of ‘months’ shadows a
global declaration [-Wshadow]
jsonpath_exec.c:410:17: warning: declaration of ‘found’ shadows a
previous local [-Wshadow]
jsonpath_exec.c:1328:24: warning: declaration of ‘res’ shadows a
previous local [-Wshadow]
jsonpath_exec.c:1339:24: warning: declaration of ‘res’ shadows a
previous local [-Wshadow]
jsonpath_exec.c:1515:17: warning: declaration of ‘res’ shadows a
previous local [-Wshadow]
pg_upgrade_support.c:220:13: warning: declaration of ‘extName’ shadows
a previous local [-Wshadow]
timestamp.c:1503:9: warning: declaration of ‘months’ shadows a global
declaration [-Wshadow]
timestamp.c:1505:9: warning: declaration of ‘days’ shadows a global
declaration [-Wshadow]
timestamp.c:2401:9: warning: declaration of ‘days’ shadows a global
declaration [-Wshadow]
timestamp.c:5345:11: warning: declaration of ‘val’ shadows a previous
local [-Wshadow]
varlena.c:4177:13: warning: declaration of ‘chunk_start’ shadows a
previous local [-Wshadow]
varlena.c:5764:14: warning: declaration of ‘str’ shadows a previous
local [-Wshadow]
inval.c:361:31: warning: declaration of ‘msg’ shadows a previous local
[-Wshadow]
inval.c:361:31: warning: declaration of ‘msg’ shadows a previous local
[-Wshadow]
inval.c:377:31: warning: declaration of ‘msgs’ shadows a parameter [-Wshadow]
inval.c:377:31: warning: declaration of ‘msgs’ shadows a parameter [-Wshadow]
inval.c:377:31: warning: declaration of ‘msgs’ shadows a parameter [-Wshadow]
inval.c:377:31: warning: declaration of ‘msgs’ shadows a parameter [-Wshadow]
freepage.c:1589:9: warning: declaration of ‘result’ shadows a previous
local [-Wshadow]
fe-connect.c:3572:11: warning: declaration of ‘res’ shadows a previous
local [-Wshadow]
fe-secure-openssl.c:1527:15: warning: declaration of ‘err’ shadows a
previous local [-Wshadow]
execute.c:111:19: warning: declaration of ‘text’ shadows a global
declaration [-Wshadow]
prepare.c:104:26: warning: declaration of ‘text’ shadows a global
declaration [-Wshadow]
prepare.c:270:12: warning: declaration of ‘text’ shadows a global
declaration [-Wshadow]
c_keywords.c:36:32: warning: declaration of ‘text’ shadows a global
declaration [-Wshadow]
descriptor.c:76:34: warning: declaration of ‘connection’ shadows a
global declaration [-Wshadow]
descriptor.c:99:35: warning: declaration of ‘connection’ shadows a
global declaration [-Wshadow]
descriptor.c:131:37: warning: declaration of ‘connection’ shadows a
global declaration [-Wshadow]
ecpg_keywords.c:39:35: warning: declaration of ‘text’ shadows a global
declaration [-Wshadow]
preproc.y:244:46: warning: declaration of ‘cur’ shadows a global
declaration [-Wshadow]
preproc.y:535:48: warning: declaration of ‘initializer’ shadows a
global declaration [-Wshadow]
print.c:956:10: warning: declaration of ‘curr_nl_line’ shadows a
previous local [-Wshadow]
initdb.c:750:14: warning: declaration of ‘username’ shadows a global
declaration [-Wshadow]
initdb.c:980:11: warning: declaration of ‘conf_file’ shadows a global
declaration [-Wshadow]
initdb.c:2069:31: warning: declaration of ‘locale’ shadows a global
declaration [-Wshadow]
initdb.c:2132:45: warning: declaration of ‘locale’ shadows a global
declaration [-Wshadow]
initdb.c:2193:35: warning: declaration of ‘locale’ shadows a global
declaration [-Wshadow]
initdb.c:2430:19: warning: declaration of ‘progname’ shadows a global
declaration [-Wshadow]
initdb.c:2509:33: warning: declaration of ‘authmethodlocal’ shadows a
global declaration [-Wshadow]
initdb.c:2509:62: warning: declaration of ‘authmethodhost’ shadows a
global declaration [-Wshadow]
pg_amcheck.c:1136:18: warning: declaration of ‘progname’ shadows a
global declaration [-Wshadow]
pg_basebackup.c:987:25: warning: declaration of ‘conn’ shadows a
global declaration [-Wshadow]
pg_basebackup.c:1257:30: warning: declaration of ‘conn’ shadows a
global declaration [-Wshadow]
pg_basebackup.c:1572:24: warning: declaration of ‘conn’ shadows a
global declaration [-Wshadow]
pg_basebackup.c:1671:31: warning: declaration of ‘conn’ shadows a
global declaration [-Wshadow]
pg_basebackup.c:1708:39: warning: declaration of ‘conn’ shadows a
global declaration [-Wshadow]
receivelog.c:337:22: warning: declaration of ‘conn’ shadows a global
declaration [-Wshadow]
receivelog.c:375:40: warning: declaration of ‘conn’ shadows a global
declaration [-Wshadow]
receivelog.c:453:27: warning: declaration of ‘conn’ shadows a global
declaration [-Wshadow]
receivelog.c:745:26: warning: declaration of ‘conn’ shadows a global
declaration [-Wshadow]
receivelog.c:870:24: warning: declaration of ‘conn’ shadows a global
declaration [-Wshadow]
receivelog.c:932:27: warning: declaration of ‘conn’ shadows a global
declaration [-Wshadow]
receivelog.c:986:29: warning: declaration of ‘conn’ shadows a global
declaration [-Wshadow]
receivelog.c:1040:28: warning: declaration of ‘conn’ shadows a global
declaration [-Wshadow]
receivelog.c:1171:31: warning: declaration of ‘conn’ shadows a global
declaration [-Wshadow]
receivelog.c:1211:29: warning: declaration of ‘conn’ shadows a global
declaration [-Wshadow]
streamutil.c:268:28: warning: declaration of ‘conn’ shadows a global
declaration [-Wshadow]
streamutil.c:346:35: warning: declaration of ‘conn’ shadows a global
declaration [-Wshadow]
streamutil.c:400:27: warning: declaration of ‘conn’ shadows a global
declaration [-Wshadow]
streamutil.c:481:28: warning: declaration of ‘conn’ shadows a global
declaration [-Wshadow]
streamutil.c:575:31: warning: declaration of ‘conn’ shadows a global
declaration [-Wshadow]
streamutil.c:683:29: warning: declaration of ‘conn’ shadows a global
declaration [-Wshadow]
pg_receivewal.c:280:11: warning: declaration of ‘tli’ shadows a
parameter [-Wshadow]
pg_recvlogical.c:126:22: warning: declaration of ‘conn’ shadows a
global declaration [-Wshadow]
pg_recvlogical.c:1025:30: warning: declaration of ‘conn’ shadows a
global declaration [-Wshadow]
pg_recvlogical.c:1042:28: warning: declaration of ‘conn’ shadows a
global declaration [-Wshadow]
pg_recvlogical.c:1042:45: warning: declaration of ‘endpos’ shadows a
global declaration [-Wshadow]
pg_controldata.c:73:24: warning: declaration of ‘wal_level’ shadows a
global declaration [-Wshadow]
pg_ctl.c:868:36: warning: declaration of ‘argv0’ shadows a global
declaration [-Wshadow]
pg_dump.c:1054:18: warning: declaration of ‘progname’ shadows a global
declaration [-Wshadow]
pg_dump.c:1380:13: warning: declaration of ‘strict_names’ shadows a
global declaration [-Wshadow]
pg_dump.c:1439:16: warning: declaration of ‘strict_names’ shadows a
global declaration [-Wshadow]
pg_dump.c:1543:15: warning: declaration of ‘strict_names’ shadows a
global declaration [-Wshadow]
pg_dump.c:9879:15: warning: declaration of ‘comments’ shadows a global
declaration [-Wshadow]
pg_dump.c:9880:8: warning: declaration of ‘ncomments’ shadows a global
declaration [-Wshadow]
pg_dump.c:9992:15: warning: declaration of ‘comments’ shadows a global
declaration [-Wshadow]
pg_dump.c:9993:8: warning: declaration of ‘ncomments’ shadows a global
declaration [-Wshadow]
pg_dump.c:11661:15: warning: declaration of ‘comments’ shadows a
global declaration [-Wshadow]
pg_dump.c:11662:8: warning: declaration of ‘ncomments’ shadows a
global declaration [-Wshadow]
pg_restore.c:430:19: warning: declaration of ‘progname’ shadows a
global declaration [-Wshadow]
pg_dumpall.c:1846:11: warning: declaration of ‘connstr’ shadows a
global declaration [-Wshadow]
pg_resetwal.c:709:25: warning: declaration of ‘guessed’ shadows a
global declaration [-Wshadow]
file_ops.c:189:8: warning: declaration of ‘dstpath’ shadows a global
declaration [-Wshadow]
file_ops.c:208:8: warning: declaration of ‘dstpath’ shadows a global
declaration [-Wshadow]
file_ops.c:231:8: warning: declaration of ‘dstpath’ shadows a global
declaration [-Wshadow]
file_ops.c:245:8: warning: declaration of ‘dstpath’ shadows a global
declaration [-Wshadow]
file_ops.c:259:8: warning: declaration of ‘dstpath’ shadows a global
declaration [-Wshadow]
file_ops.c:273:8: warning: declaration of ‘dstpath’ shadows a global
declaration [-Wshadow]
pg_rewind.c:90:19: warning: declaration of ‘progname’ shadows a global
declaration [-Wshadow]
pg_rewind.c:541:51: warning: declaration of ‘source’ shadows a global
declaration [-Wshadow]
xlogreader.c:108:24: warning: declaration of ‘wal_segment_size’
shadows a global declaration [-Wshadow]
pg_test_fsync.c:621:29: warning: declaration of ‘start_t’ shadows a
global declaration [-Wshadow]
pg_test_fsync.c:621:53: warning: declaration of ‘stop_t’ shadows a
global declaration [-Wshadow]
xlogreader.c:108:24: warning: declaration of ‘wal_segment_size’
shadows a global declaration [-Wshadow]
pgbench.c:4536:11: warning: declaration of ‘skipped’ shadows a
parameter [-Wshadow]
common.c:1912:9: warning: declaration of ‘buf’ shadows a previous
local [-Wshadow]
describe.c:1719:17: warning: declaration of ‘myopt’ shadows a previous
local [-Wshadow]
describe.c:2243:13: warning: declaration of ‘schemaname’ shadows a
parameter [-Wshadow]
prompt.c:340:12: warning: declaration of ‘p’ shadows a previous local [-Wshadow]
tab-complete.c:1630:29: warning: declaration of ‘text’ shadows a
global declaration [-Wshadow]
tab-complete.c:4979:46: warning: declaration of ‘text’ shadows a
global declaration [-Wshadow]
tab-complete.c:5008:38: warning: declaration of ‘text’ shadows a
global declaration [-Wshadow]
tab-complete.c:5017:36: warning: declaration of ‘text’ shadows a
global declaration [-Wshadow]
tab-complete.c:5026:37: warning: declaration of ‘text’ shadows a
global declaration [-Wshadow]
tab-complete.c:5037:33: warning: declaration of ‘text’ shadows a
global declaration [-Wshadow]
tab-complete.c:5045:43: warning: declaration of ‘text’ shadows a
global declaration [-Wshadow]
tab-complete.c:5061:40: warning: declaration of ‘text’ shadows a
global declaration [-Wshadow]
tab-complete.c:5069:50: warning: declaration of ‘text’ shadows a
global declaration [-Wshadow]
tab-complete.c:5131:19: warning: declaration of ‘text’ shadows a
global declaration [-Wshadow]
tab-complete.c:5502:32: warning: declaration of ‘text’ shadows a
global declaration [-Wshadow]
tab-complete.c:5582:33: warning: declaration of ‘text’ shadows a
global declaration [-Wshadow]
tab-complete.c:5630:37: warning: declaration of ‘text’ shadows a
global declaration [-Wshadow]
tab-complete.c:5675:33: warning: declaration of ‘text’ shadows a
global declaration [-Wshadow]
tab-complete.c:5807:27: warning: declaration of ‘text’ shadows a
global declaration [-Wshadow]