[libvirt] [PATCH] qemu: Use 'idx' instead of 'index' for variable name

Apparently for Xen-devel 'index' is a global and causes a build failure, so just use the shortened 'idx' instead to avoid the conflict. Signed-off-by: John Ferlan <jferlan@redhat.com> --- I did try modify cfg.mk to add a syntax check similar to the ii,jj,kk checks, but it tripped up over 'index' declarations inside structures. I'd be happy to add one, but I need some help formatting it... Here's what I had: sc_prohibit_int_index: @prohibit='\<(int|unsigned) ([^(=]* )*(index)\>(\s|,|;)' \ halt='use different name than 'index' for declaration' \ $(_sc_search_regexp) src/qemu/qemu_driver.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index e790664..3510690 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4638,7 +4638,7 @@ static void qemuProcessEventHandler(void *data, void *opaque) static virCgroupPtr qemuDomainAddCgroupForThread(virCgroupPtr cgroup, virCgroupThreadName nameval, - int index, + int idx, char *mem_mask, pid_t pid) { @@ -4646,7 +4646,7 @@ qemuDomainAddCgroupForThread(virCgroupPtr cgroup, int rv = -1; /* Create cgroup */ - if (virCgroupNewThread(cgroup, nameval, index, true, &new_cgroup) < 0) + if (virCgroupNewThread(cgroup, nameval, idx, true, &new_cgroup) < 0) return NULL; if (mem_mask && virCgroupSetCpusetMems(new_cgroup, mem_mask) < 0) @@ -4657,7 +4657,7 @@ qemuDomainAddCgroupForThread(virCgroupPtr cgroup, if (rv < 0) { virReportSystemError(-rv, _("unable to add id %d task %d to cgroup"), - index, pid); + idx, pid); virCgroupRemove(new_cgroup); goto error; } @@ -4671,7 +4671,7 @@ qemuDomainAddCgroupForThread(virCgroupPtr cgroup, static int qemuDomainHotplugAddPin(virBitmapPtr cpumask, - int index, + int idx, virDomainPinDefPtr **pindef_list, size_t *npin) { @@ -4685,7 +4685,7 @@ qemuDomainHotplugAddPin(virBitmapPtr cpumask, VIR_FREE(pindef); goto cleanup; } - pindef->id = index; + pindef->id = idx; if (VIR_APPEND_ELEMENT_COPY(*pindef_list, *npin, pindef) < 0) { virBitmapFree(pindef->cpumask); VIR_FREE(pindef); @@ -4699,7 +4699,7 @@ qemuDomainHotplugAddPin(virBitmapPtr cpumask, static int qemuDomainHotplugPinThread(virBitmapPtr cpumask, - int index, + int idx, pid_t pid, virCgroupPtr cgroup) { @@ -4709,14 +4709,14 @@ qemuDomainHotplugPinThread(virBitmapPtr cpumask, if (qemuSetupCgroupCpusetCpus(cgroup, cpumask) < 0) { virReportError(VIR_ERR_OPERATION_INVALID, _("failed to set cpuset.cpus in cgroup for id %d"), - index); + idx); goto cleanup; } } else { if (virProcessSetAffinity(pid, cpumask) < 0) { virReportError(VIR_ERR_SYSTEM_ERROR, _("failed to set cpu affinity for id %d"), - index); + idx); goto cleanup; } } @@ -4730,12 +4730,12 @@ qemuDomainHotplugPinThread(virBitmapPtr cpumask, static int qemuDomainDelCgroupForThread(virCgroupPtr cgroup, virCgroupThreadName nameval, - int index) + int idx) { virCgroupPtr new_cgroup = NULL; if (cgroup) { - if (virCgroupNewThread(cgroup, nameval, index, false, &new_cgroup) < 0) + if (virCgroupNewThread(cgroup, nameval, idx, false, &new_cgroup) < 0) return -1; /* Remove the offlined cgroup */ -- 2.1.0

On Tue, Apr 14, 2015 at 06:45:48AM -0400, John Ferlan wrote:
Apparently for Xen-devel 'index' is a global and causes a build failure, so just use the shortened 'idx' instead to avoid the conflict.
Signed-off-by: John Ferlan <jferlan@redhat.com> ---
I did try modify cfg.mk to add a syntax check similar to the ii,jj,kk checks, but it tripped up over 'index' declarations inside structures. I'd be happy to add one, but I need some help formatting it... Here's what I had:
sc_prohibit_int_index: @prohibit='\<(int|unsigned) ([^(=]* )*(index)\>(\s|,|;)' \ halt='use different name than 'index' for declaration' \ $(_sc_search_regexp)
We could just rename the fields in the structs too - wouldn't be the end of the world to use idx in those too just to simplify our life. ACK to your immediate fix now anyway. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

On 04/14/2015 06:51 AM, Daniel P. Berrange wrote:
On Tue, Apr 14, 2015 at 06:45:48AM -0400, John Ferlan wrote:
Apparently for Xen-devel 'index' is a global and causes a build failure, so just use the shortened 'idx' instead to avoid the conflict.
Signed-off-by: John Ferlan <jferlan@redhat.com> ---
I did try modify cfg.mk to add a syntax check similar to the ii,jj,kk checks, but it tripped up over 'index' declarations inside structures. I'd be happy to add one, but I need some help formatting it... Here's what I had:
sc_prohibit_int_index: @prohibit='\<(int|unsigned) ([^(=]* )*(index)\>(\s|,|;)' \ halt='use different name than 'index' for declaration' \ $(_sc_search_regexp)
We could just rename the fields in the structs too - wouldn't be the end of the world to use idx in those too just to simplify our life.
ACK to your immediate fix now anyway.
Regards, Daniel
I thought of that too, but it picked up a few comments too ... src/libvirt-domain.c:11105: * "block.<num>.backingIndex" - unsigned int giving the <backingStore> index, ... src/util/virnetdev.c:870: * @ifindex: Pointer to int where the index will be written into So rather than "wait" I figured get this out there now and see what feedback I can get to make a rule... Thanks for the quick review - I'll push what I have and generate something for the other 'index' uses. John

On Tue, Apr 14, 2015 at 07:19:17AM -0400, John Ferlan wrote:
On 04/14/2015 06:51 AM, Daniel P. Berrange wrote:
On Tue, Apr 14, 2015 at 06:45:48AM -0400, John Ferlan wrote:
Apparently for Xen-devel 'index' is a global and causes a build failure, so just use the shortened 'idx' instead to avoid the conflict.
Signed-off-by: John Ferlan <jferlan@redhat.com> ---
I did try modify cfg.mk to add a syntax check similar to the ii,jj,kk checks, but it tripped up over 'index' declarations inside structures. I'd be happy to add one, but I need some help formatting it... Here's what I had:
sc_prohibit_int_index: @prohibit='\<(int|unsigned) ([^(=]* )*(index)\>(\s|,|;)' \ halt='use different name than 'index' for declaration' \ $(_sc_search_regexp)
We could just rename the fields in the structs too - wouldn't be the end of the world to use idx in those too just to simplify our life.
ACK to your immediate fix now anyway.
Regards, Daniel
I thought of that too, but it picked up a few comments too
... src/libvirt-domain.c:11105: * "block.<num>.backingIndex" - unsigned int giving the <backingStore> index, ... src/util/virnetdev.c:870: * @ifindex: Pointer to int where the index will be written into
How about changing the regex to (int|unsigned)\s*\*?index That'd probably have less false positives and still catch the common cases well enough Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

On 04/14/2015 07:24 AM, Daniel P. Berrange wrote:
On Tue, Apr 14, 2015 at 07:19:17AM -0400, John Ferlan wrote:
I thought of that too, but it picked up a few comments too
... src/libvirt-domain.c:11105: * "block.<num>.backingIndex" - unsigned int giving the <backingStore> index, ... src/util/virnetdev.c:870: * @ifindex: Pointer to int where the index will be written into
How about changing the regex to
(int|unsigned)\s*\*?index
That'd probably have less false positives and still catch the common cases well enough
That worked! Thanks - regex's are still mostly a mystery I've put together a series which I'll post shortly John
participants (2)
-
Daniel P. Berrange
-
John Ferlan