[libvirt] [PATCH] blockjob: avoid 32-bit compilation warning
Commit c1d75de caused this warning on 32-bit platforms (fatal when -Werror is enabled): virsh-domain.c: In function 'cmdBlockCopy': virsh-domain.c:2003:17: error: comparison is always false due to limited range of data type [-Werror=type-limits] Forcing the left side of the < to be ull instead of ul shuts up the 32-bit compiler while still protecting 64-bit code from overflow. * tools/virsh-domain.c (cmdBlockCopy): Add type coercion. Signed-off-by: Eric Blake <eblake@redhat.com> --- Pushing under the build-breaker rule. tools/virsh-domain.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 39ff798..ed2e3ea 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -2000,7 +2000,7 @@ cmdBlockCopy(vshControl *ctl, const vshCmd *cmd) if (bandwidth) { /* bandwidth is ulong MiB/s, but the typed parameter is * ullong bytes/s; make sure we don't overflow */ - if (bandwidth > ULLONG_MAX >> 20) { + if (bandwidth + 0ULL > ULLONG_MAX >> 20) { virReportError(VIR_ERR_OVERFLOW, _("bandwidth must be less than %llu"), ULLONG_MAX >> 20); -- 1.9.3
On 09/08/2014 08:55 AM, Eric Blake wrote:
Forcing the left side of the < to be ull instead of ul shuts up the 32-bit compiler while still protecting 64-bit code from overflow.
* ullong bytes/s; make sure we don't overflow */ - if (bandwidth > ULLONG_MAX >> 20) { + if (bandwidth + 0ULL > ULLONG_MAX >> 20) {
On IRC, several people suggested that I write this with an explicit cast: if ((unsigned long long) bandwidth > ULLONG_MAX >> 20) { rather than my shorthand of an identity operation used to coerce types via promotion rules. If you feel strongly in favor of one of the two approaches, or strongly about avoiding needless churn now that the patch is already pushed, speak up now; if not, I'll probably make the change to use an explicit cast later today. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 09/08/14 17:43, Eric Blake wrote:
On 09/08/2014 08:55 AM, Eric Blake wrote:
Forcing the left side of the < to be ull instead of ul shuts up the 32-bit compiler while still protecting 64-bit code from overflow.
* ullong bytes/s; make sure we don't overflow */ - if (bandwidth > ULLONG_MAX >> 20) { + if (bandwidth + 0ULL > ULLONG_MAX >> 20) {
On IRC, several people suggested that I write this with an explicit cast:
if ((unsigned long long) bandwidth > ULLONG_MAX >> 20) {
rather than my shorthand of an identity operation used to coerce types via promotion rules. If you feel strongly in favor of one of the two approaches, or strongly about avoiding needless churn now that the patch is already pushed, speak up now; if not, I'll probably make the change to use an explicit cast later today.
I don't think it's worth fixing up since it's pushed already. Peter
participants (2)
-
Eric Blake -
Peter Krempa