On 10/16/2012 08:05 AM, Viktor Mihajlovski wrote:
New macro VIR_CPU_USED added to facilitate the interpretation of cpu maps. Further, hardened the other cpumap macros against invocations like VIR_CPU_USE(cpumap + 1, cpu)
Signed-off-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com> --- include/libvirt/libvirt.h.in | 20 ++++++++++++++++---- 1 files changed, 16 insertions(+), 4 deletions(-)
Hmm - should we rewrite VIR_CPU_USABLE in terms of VIR_CPU_USED()? You also missed hardening VIR_COPY_CPUMAP and VIR_GET_CPUMAP.
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 6b72159..3e8e4f8 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -1954,7 +1954,7 @@ int virDomainGetEmulatorPinInfo (virDomainPtr domain, * USE_CPU macro set the bit (CPU usable) of the related cpu in cpumap.
While we are touching here, lets fix the poor grammar: s/USE_CPU macro set/It sets/
*/
-#define VIR_USE_CPU(cpumap,cpu) (cpumap[(cpu)/8] |= (1<<((cpu)%8))) +#define VIR_USE_CPU(cpumap,cpu) ((cpumap)[(cpu)/8] |= (1<<((cpu)%8)))
As long as we are touching this, we might as well use conventional spacing (space after comma and around operators).
/** * VIR_UNUSE_CPU: @@ -1965,7 +1965,19 @@ int virDomainGetEmulatorPinInfo (virDomainPtr domain, * USE_CPU macro reset the bit (CPU not usable) of the related cpu in cpumap.
Again, bad grammar.
*/
-#define VIR_UNUSE_CPU(cpumap,cpu) (cpumap[(cpu)/8] &= ~(1<<((cpu)%8))) +#define VIR_UNUSE_CPU(cpumap,cpu) ((cpumap)[(cpu)/8] &= ~(1<<((cpu)%8)))
And again, odd spacing.
+ +/** + * VIR_CPU_USED: + * @cpumap: pointer to a bit map of real CPUs (in 8-bit bytes) (IN) + * @cpu: the physical CPU number + * + * This macro can be used in conjunction with virNodeGetCPUMapFlags() API. + * VIR_CPU_USED returns true if the bit of the related CPU is set.
Must it return true (== 1), or should we document merely that it returns non-zero...
+ * + */ + +#define VIR_CPU_USED(cpumap,cpu) ((cpumap)[(cpu)/8] & (1<<((cpu)%8)))
...given that you aren't returning a bool here? But VIR_CPU_USABLE didn't bother converting to bool, so this is just a docs issue.
/** * VIR_CPU_MAPLEN: @@ -1976,7 +1988,7 @@ int virDomainGetEmulatorPinInfo (virDomainPtr domain, * CPU map between a single virtual & all physical CPUs of a domain. */
-#define VIR_CPU_MAPLEN(cpu) (((cpu)+7)/8) +#define VIR_CPU_MAPLEN(cpu) (((cpu)+7)/8)
Why the whitespace change? But we might as well fix operator spacing.
int virDomainGetVcpus (virDomainPtr domain, @@ -1998,7 +2010,7 @@ int virDomainGetVcpus (virDomainPtr domain, */
#define VIR_CPU_USABLE(cpumaps,maplen,vcpu,cpu) \ - (cpumaps[((vcpu)*(maplen))+((cpu)/8)] & (1<<((cpu)%8))) + ((cpumaps)[((vcpu)*(maplen))+((cpu)/8)] & (1<<((cpu)%8)))
/** * VIR_COPY_CPUMAP:
Here's what I'm squashing: diff --git i/include/libvirt/libvirt.h.in w/include/libvirt/libvirt.h.in index adf5afb..428db39 100644 --- i/include/libvirt/libvirt.h.in +++ w/include/libvirt/libvirt.h.in @@ -2045,10 +2045,10 @@ int virDomainGetEmulatorPinInfo (virDomainPtr domain, * @cpu: the physical CPU number * * This macro is to be used in conjunction with virDomainPinVcpu() API. - * USE_CPU macro set the bit (CPU usable) of the related cpu in cpumap. + * It sets the bit (CPU usable) of the related cpu in cpumap. */ -#define VIR_USE_CPU(cpumap,cpu) ((cpumap)[(cpu)/8] |= (1<<((cpu)%8))) +#define VIR_USE_CPU(cpumap, cpu) ((cpumap)[(cpu) / 8] |= (1 << ((cpu) % 8))) /** * VIR_UNUSE_CPU: @@ -2056,10 +2056,10 @@ int virDomainGetEmulatorPinInfo (virDomainPtr domain, * @cpu: the physical CPU number * * This macro is to be used in conjunction with virDomainPinVcpu() API. - * USE_CPU macro reset the bit (CPU not usable) of the related cpu in cpumap. + * It resets the bit (CPU not usable) of the related cpu in cpumap. */ -#define VIR_UNUSE_CPU(cpumap,cpu) ((cpumap)[(cpu)/8] &= ~(1<<((cpu)%8))) +#define VIR_UNUSE_CPU(cpumap, cpu) ((cpumap)[(cpu) / 8] &= ~(1 << ((cpu) % 8))) /** * VIR_CPU_USED: @@ -2067,11 +2067,10 @@ int virDomainGetEmulatorPinInfo (virDomainPtr domain, * @cpu: the physical CPU number * * This macro can be used in conjunction with virNodeGetCPUMapFlags() API. - * VIR_CPU_USED returns true if the bit of the related CPU is set. - * + * It returns non-zero if the bit of the related CPU is set. */ -#define VIR_CPU_USED(cpumap,cpu) ((cpumap)[(cpu)/8] & (1<<((cpu)%8))) +#define VIR_CPU_USED(cpumap, cpu) ((cpumap)[(cpu) / 8] & (1 << ((cpu) % 8))) /** * VIR_CPU_MAPLEN: @@ -2082,7 +2081,7 @@ int virDomainGetEmulatorPinInfo (virDomainPtr domain, * CPU map between a single virtual & all physical CPUs of a domain. */ -#define VIR_CPU_MAPLEN(cpu) (((cpu)+7)/8) +#define VIR_CPU_MAPLEN(cpu) (((cpu) + 7) / 8) int virDomainGetVcpus (virDomainPtr domain, @@ -2099,12 +2098,12 @@ int virDomainGetVcpus (virDomainPtr domain, * @cpu: the physical CPU number * * This macro is to be used in conjunction with virDomainGetVcpus() API. - * VIR_CPU_USABLE macro returns a non zero value (true) if the cpu + * VIR_CPU_USABLE macro returns a non-zero value (true) if the cpu * is usable by the vcpu, and 0 otherwise. */ -#define VIR_CPU_USABLE(cpumaps,maplen,vcpu,cpu) \ - ((cpumaps)[((vcpu)*(maplen))+((cpu)/8)] & (1<<((cpu)%8))) +#define VIR_CPU_USABLE(cpumaps, maplen, vcpu, cpu) \ + VIR_CPU_USED(VIR_GET_CPUMAP(cpumaps, maplen, vcpu), cpu) /** * VIR_COPY_CPUMAP: @@ -2116,12 +2115,12 @@ int virDomainGetVcpus (virDomainPtr domain, * (ie: malloc(maplen)) * * This macro is to be used in conjunction with virDomainGetVcpus() and - * virDomainPinVcpu() APIs. VIR_COPY_CPUMAP macro extract the cpumap of - * the specified vcpu from cpumaps array and copy it into cpumap to be used + * virDomainPinVcpu() APIs. VIR_COPY_CPUMAP macro extracts the cpumap of + * the specified vcpu from cpumaps array and copies it into cpumap to be used * later by virDomainPinVcpu() API. */ -#define VIR_COPY_CPUMAP(cpumaps,maplen,vcpu,cpumap) \ - memcpy(cpumap, &(cpumaps[(vcpu)*(maplen)]), (maplen)) +#define VIR_COPY_CPUMAP(cpumaps, maplen, vcpu, cpumap) \ + memcpy(cpumap, VIR_GET_CPUMAP(cpumaps, maplen, vcpu), maplen) /** @@ -2134,7 +2133,7 @@ int virDomainGetVcpus (virDomainPtr domain, * virDomainPinVcpu() APIs. VIR_GET_CPUMAP macro returns a pointer to the * cpumap of the specified vcpu from cpumaps array. */ -#define VIR_GET_CPUMAP(cpumaps,maplen,vcpu) &(cpumaps[(vcpu)*(maplen)]) +#define VIR_GET_CPUMAP(cpumaps, maplen, vcpu) (&((cpumaps)[(vcpu) * (maplen)])) typedef enum { -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org