On 10/11/2013 12:50 AM, mars@xxxxxxxxxxxxxxxxxx wrote:
> From: Bing Bu Cao <mars@xxxxxxxxxxxxxxxxxx>
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@xxxxxxxxxxxxxxxxxx>
> ---
> 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@xxxxxxxxxxxxxxxxxx>
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@xxxxxxxxxxxxxxxxxx>
Signed-off-by: Eric Blake <eblake@xxxxxxxxxx>
---
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
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list