[libvirt] [PATCH 0/5] Prohibit use of 'index' for variable

Given the recent Xen-devel build failure/issue - this series of patches will remove our uses of '[unsigned] int index' and then generate a rule for it John Ferlan (5): qemu: Adjust the prototype to match the function util: Use 'idx' instead of 'index' snapshot: Change 'index' to 'idx' nwfilter: Change 'index' to 'idx' cfg: Disallow usage of 'index' as variable name cfg.mk | 5 +++++ src/conf/nwfilter_params.c | 8 ++++---- src/conf/nwfilter_params.h | 2 +- src/conf/snapshot_conf.c | 6 +++--- src/conf/snapshot_conf.h | 2 +- src/qemu/qemu_conf.h | 2 +- src/qemu/qemu_driver.c | 4 ++-- src/util/virnetdev.c | 4 ++-- 8 files changed, 19 insertions(+), 14 deletions(-) -- 2.1.0

Changing the prototype to not have "int *index" since we'll soon be disallowing index as a name. Curiously the original commit (a4504ac) for the function used 'int idx' in the function - so they didn't match. Now they do. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_conf.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index cb01fb6..4b02d19 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -288,7 +288,7 @@ typedef qemuSharedDeviceEntry *qemuSharedDeviceEntryPtr; bool qemuSharedDeviceEntryDomainExists(qemuSharedDeviceEntryPtr entry, const char *name, - int *index) + int *idx) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); char *qemuGetSharedDeviceKey(const char *disk_path) -- 2.1.0

Impending syntax checker will disallow 'int index', so change it here. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/util/virnetdev.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c index a7903c3..9a6d4e7 100644 --- a/src/util/virnetdev.c +++ b/src/util/virnetdev.c @@ -100,7 +100,7 @@ typedef enum { typedef struct _virNetDevMcastEntry virNetDevMcastEntry; typedef virNetDevMcastEntry *virNetDevMcastEntryPtr; struct _virNetDevMcastEntry { - int index; + int idx; char name[VIR_MCAST_NAME_LEN]; int users; bool global; @@ -2605,7 +2605,7 @@ static int virNetDevParseMcast(char *buf, virNetDevMcastEntryPtr mcast) return -1; } - mcast->index = num; + mcast->idx = num; break; case VIR_MCAST_TYPE_NAME_TOKEN: if (virStrncpy(mcast->name, token, strlen(token), -- 2.1.0

Forthcoming syntax check rule will disallow usage of 'int index', so change it for snapshot Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/conf/snapshot_conf.c | 6 +++--- src/conf/snapshot_conf.h | 2 +- src/qemu/qemu_driver.c | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c index ca6796a..df19449 100644 --- a/src/conf/snapshot_conf.c +++ b/src/conf/snapshot_conf.c @@ -440,7 +440,7 @@ disksorter(const void *a, const void *b) const virDomainSnapshotDiskDef *diskb = b; /* Integer overflow shouldn't be a problem here. */ - return diska->index - diskb->index; + return diska->idx - diskb->idx; } /* Align def->disks to def->domain. Sort the list of def->disks, @@ -500,7 +500,7 @@ virDomainSnapshotAlignDisks(virDomainSnapshotDefPtr def, goto cleanup; } ignore_value(virBitmapSetBit(map, idx)); - disk->index = idx; + disk->idx = idx; disk_snapshot = def->dom->disks[idx]->snapshot; if (!disk->snapshot) { @@ -553,7 +553,7 @@ virDomainSnapshotAlignDisks(virDomainSnapshotDefPtr def, goto cleanup; if (VIR_STRDUP(disk->name, def->dom->disks[i]->dst) < 0) goto cleanup; - disk->index = i; + disk->idx = i; /* Don't snapshot empty drives */ if (virStorageSourceIsEmpty(def->dom->disks[i]->src)) diff --git a/src/conf/snapshot_conf.h b/src/conf/snapshot_conf.h index ec3a0ea..167b27f 100644 --- a/src/conf/snapshot_conf.h +++ b/src/conf/snapshot_conf.h @@ -49,7 +49,7 @@ typedef struct _virDomainSnapshotDiskDef virDomainSnapshotDiskDef; typedef virDomainSnapshotDiskDef *virDomainSnapshotDiskDefPtr; struct _virDomainSnapshotDiskDef { char *name; /* name matching the <target dev='...' of the domain */ - int index; /* index within snapshot->dom->disks that matches name */ + int idx; /* index within snapshot->dom->disks that matches name */ int snapshot; /* virDomainSnapshotLocation */ /* details of wrapper external file. src is always non-NULL. diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 3510690..8589681 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -13567,7 +13567,7 @@ qemuDomainSnapshotCreateInactiveExternal(virQEMUDriverPtr driver, * create them correctly. */ for (i = 0; i < snap->def->ndisks && !reuse; i++) { snapdisk = &(snap->def->disks[i]); - defdisk = snap->def->dom->disks[snapdisk->index]; + defdisk = snap->def->dom->disks[snapdisk->idx]; if (snapdisk->snapshot != VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL) continue; @@ -13618,7 +13618,7 @@ qemuDomainSnapshotCreateInactiveExternal(virQEMUDriverPtr driver, /* update disk definitions */ for (i = 0; i < snap->def->ndisks; i++) { snapdisk = &(snap->def->disks[i]); - defdisk = vm->def->disks[snapdisk->index]; + defdisk = vm->def->disks[snapdisk->idx]; if (snapdisk->snapshot == VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL) { VIR_FREE(defdisk->src->path); -- 2.1.0

Forthcoming syntax check rule will disallow usage of 'int index', so change it for nwfilter Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/conf/nwfilter_params.c | 8 ++++---- src/conf/nwfilter_params.h | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/conf/nwfilter_params.c b/src/conf/nwfilter_params.c index e561943..0ac8baa 100644 --- a/src/conf/nwfilter_params.c +++ b/src/conf/nwfilter_params.c @@ -931,7 +931,7 @@ virNWFilterVarAccessEqual(const virNWFilterVarAccess *a, switch (a->accessType) { case VIR_NWFILTER_VAR_ACCESS_ELEMENT: - return (a->u.index.index == b->u.index.index && + return (a->u.index.idx == b->u.index.idx && a->u.index.intIterId == b->u.index.intIterId); break; case VIR_NWFILTER_VAR_ACCESS_ITERATOR: @@ -1010,7 +1010,7 @@ virNWFilterVarAccessParse(const char *varAccess) switch (dest->accessType) { case VIR_NWFILTER_VAR_ACCESS_ELEMENT: - dest->u.index.index = result; + dest->u.index.idx = result; dest->u.index.intIterId = ~0; break; case VIR_NWFILTER_VAR_ACCESS_ITERATOR: @@ -1044,7 +1044,7 @@ virNWFilterVarAccessPrint(virNWFilterVarAccessPtr vap, virBufferPtr buf) virBufferAdd(buf, vap->varName, -1); switch (vap->accessType) { case VIR_NWFILTER_VAR_ACCESS_ELEMENT: - virBufferAsprintf(buf, "[%u]", vap->u.index.index); + virBufferAsprintf(buf, "[%u]", vap->u.index.idx); break; case VIR_NWFILTER_VAR_ACCESS_ITERATOR: if (vap->u.iterId != 0) @@ -1076,7 +1076,7 @@ virNWFilterVarAccessGetIterId(const virNWFilterVarAccess *vap) unsigned int virNWFilterVarAccessGetIndex(const virNWFilterVarAccess *vap) { - return vap->u.index.index; + return vap->u.index.idx; } static void diff --git a/src/conf/nwfilter_params.h b/src/conf/nwfilter_params.h index 98610a7..abd5b85 100644 --- a/src/conf/nwfilter_params.h +++ b/src/conf/nwfilter_params.h @@ -111,7 +111,7 @@ struct _virNWFilterVarAccess { virNWFilterVarAccessType accessType; union { struct { - unsigned int index; + unsigned int idx; unsigned int intIterId; } index; unsigned int iterId; -- 2.1.0

Since we've run afoul of the Xen-devel build for shadowing a global declaration of 'index', just disallow using index for a variable name Signed-off-by: John Ferlan <jferlan@redhat.com> --- cfg.mk | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/cfg.mk b/cfg.mk index 3111a89..9ba2134 100644 --- a/cfg.mk +++ b/cfg.mk @@ -564,6 +564,11 @@ sc_avoid_attribute_unused_in_header: halt='use ATTRIBUTE_UNUSED in .c rather than .h files' \ $(_sc_search_regexp) +sc_prohibit_int_index: + @prohibit='\<(int|unsigned)\s*\*?index\>(\s|,|;)' \ + halt='use different name than 'index' for declaration' \ + $(_sc_search_regexp) + sc_prohibit_int_ijk: @prohibit='\<(int|unsigned) ([^(=]* )*(i|j|k)\>(\s|,|;)' \ halt='use size_t, not int/unsigned int for loop vars i, j, k' \ -- 2.1.0

On Tue, Apr 14, 2015 at 07:54:56AM -0400, John Ferlan wrote:
Given the recent Xen-devel build failure/issue - this series of patches will remove our uses of '[unsigned] int index' and then generate a rule for it
John Ferlan (5): qemu: Adjust the prototype to match the function util: Use 'idx' instead of 'index' snapshot: Change 'index' to 'idx' nwfilter: Change 'index' to 'idx' cfg: Disallow usage of 'index' as variable name
Works for me, the only left out index variable I've found out was the one visible in 4/5 which is a struct anyway and extending the check for that would (as you wrote earlier) cause a false positive in a comment. That could be changed, but isn't worth it, IMHO. ACK series.
cfg.mk | 5 +++++ src/conf/nwfilter_params.c | 8 ++++---- src/conf/nwfilter_params.h | 2 +- src/conf/snapshot_conf.c | 6 +++--- src/conf/snapshot_conf.h | 2 +- src/qemu/qemu_conf.h | 2 +- src/qemu/qemu_driver.c | 4 ++-- src/util/virnetdev.c | 4 ++-- 8 files changed, 19 insertions(+), 14 deletions(-)
-- 2.1.0
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
participants (2)
-
John Ferlan
-
Martin Kletzander