[libvirt] [PATCH] Fix: helper function virCompareLimitUlong should return -1 when the latter parameter is 0
From: Bing Bu Cao <mars@linux.vnet.ibm.com> The helper function virCompareLimitUlong compare limit values, where value of 0 is equal to unlimited, if the latter parameter is 0, it should return -1 instead of 1, hence the user can only set hard_limit when swap_hard_limit currently is unlimited. Signed-off-by: Bing Bu Cao <mars@linux.vnet.ibm.com> --- src/util/virutil.c | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-) diff --git a/src/util/virutil.c b/src/util/virutil.c index d9e0bc4..3dcf1fe 100644 --- a/src/util/virutil.c +++ b/src/util/virutil.c @@ -2067,6 +2067,9 @@ virCompareLimitUlong(unsigned long long a, unsigned long b) if (a == b) return 0; + if (0 == b) + return -1; + if (a == 0 || a > b) return 1; -- 1.7.7.6
On 10/11/2013 12:50 AM, mars@linux.vnet.ibm.com wrote:
From: Bing Bu Cao <mars@linux.vnet.ibm.com>
Long subject line. Look at 'git shortlog -30' to get a feel for how to write a concise summary. I retitled it: util: fix two virCompareLimitUlong bugs
The helper function virCompareLimitUlong compare limit values,
s/compare/compares/
where value of 0 is equal to unlimited, if the latter parameter is 0, it should return -1 instead of 1, hence the user can only set hard_limit when swap_hard_limit currently is unlimited.
Signed-off-by: Bing Bu Cao <mars@linux.vnet.ibm.com> --- src/util/virutil.c | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/src/util/virutil.c b/src/util/virutil.c index d9e0bc4..3dcf1fe 100644 --- a/src/util/virutil.c +++ b/src/util/virutil.c @@ -2067,6 +2067,9 @@ virCompareLimitUlong(unsigned long long a, unsigned long b)
Oh my. This function is BROKEN on 32-bit platforms! Comparing a 64-bit and 32-bit number, but where all callers pass 2 64-bit numbers, is prone to miscalculation if 'b' was a multiple of 4G.
if (a == b) return 0;
+ if (0 == b)
Yoda-style conditionals this is. Although some projects swear by the style of constants on the left of comparisons, we tend to avoid it in libvirt. I think 'if (!b)' is more concise anyways.
+ return -1; + if (a == 0 || a > b) return 1;
Congratulations on your first libvirt patch. ACK and pushed as follows: From 5bf6c3f20c99d2f6e93f6826a1ba158ded63bded Mon Sep 17 00:00:00 2001 From: Bing Bu Cao <mars@linux.vnet.ibm.com> Date: Fri, 11 Oct 2013 14:50:33 +0800 Subject: [PATCH] util: fix two virCompareLimitUlong bugs The helper function virCompareLimitUlong compares limit values, where value of 0 is equal to unlimited, if the latter parameter is 0, it should return -1 instead of 1, hence the user can only set hard_limit when swap_hard_limit currently is unlimited. Worse, all callers pass 2 64-bit values, but on 32-bit platforms, the second argument was silently truncated to 32 bits, which could lead to incorrect computations. Signed-off-by: Bing Bu Cao <mars@linux.vnet.ibm.com> Signed-off-by: Eric Blake <eblake@redhat.com> --- src/util/virutil.c | 5 ++++- src/util/virutil.h | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/util/virutil.c b/src/util/virutil.c index d9e0bc4..854c0ad 100644 --- a/src/util/virutil.c +++ b/src/util/virutil.c @@ -2062,11 +2062,14 @@ virFindFCHostCapableVport(const char *sysfs_prefix ATTRIBUTE_UNUSED) * Returns 0 if the numbers are equal, -1 if b is greater, 1 if a is greater. */ int -virCompareLimitUlong(unsigned long long a, unsigned long b) +virCompareLimitUlong(unsigned long long a, unsigned long long b) { if (a == b) return 0; + if (!b) + return -1; + if (a == 0 || a > b) return 1; diff --git a/src/util/virutil.h b/src/util/virutil.h index 4b06992..9f6fb20 100644 --- a/src/util/virutil.h +++ b/src/util/virutil.h @@ -168,7 +168,7 @@ char *virGetFCHostNameByWWN(const char *sysfs_prefix, char *virFindFCHostCapableVport(const char *sysfs_prefix); -int virCompareLimitUlong(unsigned long long a, unsigned long b); +int virCompareLimitUlong(unsigned long long a, unsigned long long b); int virParseOwnershipIds(const char *label, uid_t *uidPtr, gid_t *gidPtr); -- 1.8.3.1 -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
participants (2)
-
Eric Blake -
mars＠linux.vnet.ibm.com