On 02/11/2014 08:52 AM, Daniel P. Berrange wrote:
On Tue, Feb 11, 2014 at 04:39:47PM +0100, Ján Tomko wrote:
> This shadows the index function on some systems (RHEL-6.4, FreeBSD 9):
> ../../src/conf/capabilities.c: In function 'virCapabilitiesGetCpusForNode':
> ../../src/conf/capabilities.c:1005: warning: declaration of'index'
> shadows a global declaration [-Wshadow]
> /usr/include/strings.h:57: warning: shadowed declaration is here [-Wshadow]
Older gcc (on both systems you mentioned) complains about variables
overriding functions; newer gcc no longer does that. 'index' is
generally namespace pollution these days (it pre-dated POSIX, and was
replaced by strchr), but 'remove' is a real issue being standardized
with <stdio.h>.
Doh, anyone fancy coming up with a sytnax-check rule to
prevent use of 'index' and 'remove' :-)
I tried this, but it is too prone to false positives:
diff --git i/cfg.mk w/cfg.mk
index 207dfeb..c83179c 100644
--- i/cfg.mk
+++ w/cfg.mk
@@ -285,6 +285,14 @@ sc_avoid_write:
halt='consider using safewrite instead of write' \
$(_sc_search_regexp)
+# Avoid naming local variables with a name that clashes with typical
+# header pollution.
+sc_avoid_index_remove:
+ @prohibit='\<(index|remove) *[,=;]' \
+ in_vc_files='\.[hc]$$' \
+ halt='avoid "index" and "remove" as variable names' \
+ $(_sc_search_regexp)
+
# In debug statements, print flags as bitmask and mode_t as octal.
sc_flags_debug:
@prohibit='\<mode=%[0-9.]*[diux]' \
$ make sc_avoid_index_remove
avoid_index_remove
../src/conf/domain_conf.c:15099: " <controller
type='%s' index='%u'",
../src/conf/nwfilter_params.c:954: return (a->u.index.index ==
b->u.index.index &&
../src/conf/nwfilter_params.c:1033: dest->u.index.index = result;
../src/conf/nwfilter_params.c:1099: return vap->u.index.index;
../src/conf/nwfilter_params.h:117: unsigned int index;
../src/conf/nwfilter_params.h:119: } index;
../src/conf/snapshot_conf.c:427: return diska->index - diskb->index;
../src/conf/snapshot_conf.c:488: disk->index = idx;
../src/conf/snapshot_conf.c:540: disk->index = i;
../src/conf/snapshot_conf.h:52: int index; /* index within
snapshot->dom->disks that matches name */
../src/qemu/qemu_command.c:4086: virBufferAsprintf(&opt,
",index=%d", idx);
../src/qemu/qemu_command.c:10114: * eg -drive
file=/dev/HostVG/VirtData1,if=ide,index=1
../src/qemu/qemu_migration.c:567: virBufferAsprintf(buf, "
<interface index='%zu' vporttype='%s'",
../src/util/viralloc.c:476: * If *countptr <= remove, the entire array
is freed.
../src/util/virnetdev.c:1351: * @ifindex: The interface index; may be <
0 if ifname is given
../src/util/virutil.c:501: * @return name's index, or -1 on failure
../src/vbox/vbox_CAPI_v4_2_20.h:6927: PRBool remove,
../src/vbox/vbox_CAPI_v4_3.h:7654: PRBool remove,
../src/vbox/vbox_CAPI_v4_3_4.h:7654: PRBool remove,
../tests/networkxml2xmlupdatetest.c:145: DO_TEST_FULL(name,
updatexml, netxml, outxml, command, section, index, \
../tests/networkxml2xmlupdatetest.c:148: DO_TEST_FULL(name,
updatexml, netxml, "n/a", command, section, index, \
../tools/virsh.h:313:/* Given an index, return either the name of that
device (non-NULL) or
maint.mk: avoid "index" and "remove" as variable names
make: *** [sc_avoid_index_remove] Error 1
Maybe it would be simpler to just:
#define index index_is_obsolete
#include <strings.h>
#undef index
in our "internal.h" (or other common header) so that the rest of our
code can use 'index' without care.
For that matter, we are unlikely to need to call remove() (generally, we
would call unlink() or rmdir() directly because we know what file type
we are removing), so we could likewise do:
#define remove hidden_remove
#include <stdio.h>
#undef remove
Thoughts?
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library
http://libvirt.org