6:34 p.m.
Rawhide's gcc 4.5.0 got smarter in relation to Fedora 13's gcc 4.4.4,
and exposed a real bug (introduced by commit ed9c14a7 this January)
as well as extra noise that is easy enough to silence.
The new warnings looked like:
esx/esx_vi_types.c:1231:1: warning: logical 'or' of collectively exhaustive tests
is always true [-Wlogical-op]
And the esx_vi_types.c case is proof why it is a bad idea to ever
stick -Werror in a spec file - upgrade the compiler, and something
that previously built silently can now fail to build; in contrast,
using -Werror during development is great as it forces you to think
about the issue, and in the case of xen, fix a real bug.
Eric Blake (2):
xen: fix logic bug
esx: silence spurious compiler warning
src/esx/esx_vi_types.c | 6 ++++--
src/xen/xend_internal.c | 6 +++---
2 files changed, 7 insertions(+), 5 deletions(-)
--
1.7.2
6:34 p.m.
New subject: [libvirt] [PATCH 1/2] xen: fix logic bug
The recent switch to enable -Wlogical-op paid off again.
gcc 4.5.0 (rawhide) is smarter than 4.4.4 (Fedora 13).
* src/xen/xend_internal.c (xenDaemonAttachDeviceFlags)
(xenDaemonUpdateDeviceFlags, xenDaemonDetachDeviceFlags): Use
correct operator.
Signed-off-by: Eric Blake <eblake(a)redhat.com>
---
src/xen/xend_internal.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c
index fad5ce8..311775a 100644
--- a/src/xen/xend_internal.c
+++ b/src/xen/xend_internal.c
@@ -3888,7 +3888,7 @@ xenDaemonAttachDeviceFlags(virDomainPtr domain, const char *xml,
} else {
/* Only live config can be changed if xendConfigVersion < 3 */
if (priv->xendConfigVersion < 3 &&
- (flags != VIR_DOMAIN_DEVICE_MODIFY_CURRENT ||
+ (flags != VIR_DOMAIN_DEVICE_MODIFY_CURRENT &&
flags != VIR_DOMAIN_DEVICE_MODIFY_LIVE)) {
virXendError(VIR_ERR_OPERATION_INVALID, "%s",
_("Xend version does not support modifying "
@@ -4027,7 +4027,7 @@ xenDaemonUpdateDeviceFlags(virDomainPtr domain, const char *xml,
} else {
/* Only live config can be changed if xendConfigVersion < 3 */
if (priv->xendConfigVersion < 3 &&
- (flags != VIR_DOMAIN_DEVICE_MODIFY_CURRENT ||
+ (flags != VIR_DOMAIN_DEVICE_MODIFY_CURRENT &&
flags != VIR_DOMAIN_DEVICE_MODIFY_LIVE)) {
virXendError(VIR_ERR_OPERATION_INVALID, "%s",
_("Xend version does not support modifying "
@@ -4138,7 +4138,7 @@ xenDaemonDetachDeviceFlags(virDomainPtr domain, const char *xml,
} else {
/* Only live config can be changed if xendConfigVersion < 3 */
if (priv->xendConfigVersion < 3 &&
- (flags != VIR_DOMAIN_DEVICE_MODIFY_CURRENT ||
+ (flags != VIR_DOMAIN_DEVICE_MODIFY_CURRENT &&
flags != VIR_DOMAIN_DEVICE_MODIFY_LIVE)) {
virXendError(VIR_ERR_OPERATION_INVALID, "%s",
_("Xend version does not support modifying "
--
1.7.2
12:17 p.m.
New subject: [libvirt] [PATCH 1/2] xen: fix logic bug
On Wed, Jul 28, 2010 at 05:34:14PM -0600, Eric Blake wrote:
The recent switch to enable -Wlogical-op paid off again.
gcc 4.5.0 (rawhide) is smarter than 4.4.4 (Fedora 13).
* src/xen/xend_internal.c (xenDaemonAttachDeviceFlags)
(xenDaemonUpdateDeviceFlags, xenDaemonDetachDeviceFlags): Use
correct operator.
Signed-off-by: Eric Blake <eblake(a)redhat.com>
---
src/xen/xend_internal.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c
index fad5ce8..311775a 100644
--- a/src/xen/xend_internal.c
+++ b/src/xen/xend_internal.c
@@ -3888,7 +3888,7 @@ xenDaemonAttachDeviceFlags(virDomainPtr domain, const char *xml,
} else {
/* Only live config can be changed if xendConfigVersion < 3 */
if (priv->xendConfigVersion < 3 &&
- (flags != VIR_DOMAIN_DEVICE_MODIFY_CURRENT ||
+ (flags != VIR_DOMAIN_DEVICE_MODIFY_CURRENT &&
flags != VIR_DOMAIN_DEVICE_MODIFY_LIVE)) {
virXendError(VIR_ERR_OPERATION_INVALID, "%s",
_("Xend version does not support modifying "
@@ -4027,7 +4027,7 @@ xenDaemonUpdateDeviceFlags(virDomainPtr domain, const char *xml,
} else {
/* Only live config can be changed if xendConfigVersion < 3 */
if (priv->xendConfigVersion < 3 &&
- (flags != VIR_DOMAIN_DEVICE_MODIFY_CURRENT ||
+ (flags != VIR_DOMAIN_DEVICE_MODIFY_CURRENT &&
flags != VIR_DOMAIN_DEVICE_MODIFY_LIVE)) {
virXendError(VIR_ERR_OPERATION_INVALID, "%s",
_("Xend version does not support modifying "
@@ -4138,7 +4138,7 @@ xenDaemonDetachDeviceFlags(virDomainPtr domain, const char *xml,
} else {
/* Only live config can be changed if xendConfigVersion < 3 */
if (priv->xendConfigVersion < 3 &&
- (flags != VIR_DOMAIN_DEVICE_MODIFY_CURRENT ||
+ (flags != VIR_DOMAIN_DEVICE_MODIFY_CURRENT &&
flags != VIR_DOMAIN_DEVICE_MODIFY_LIVE)) {
virXendError(VIR_ERR_OPERATION_INVALID, "%s",
_("Xend version does not support modifying "
ACK,
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
daniel(a)veillard.com | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
2:53 p.m.
New subject: [libvirt] [PATCH 1/2] xen: fix logic bug
On 07/29/2010 11:17 AM, Daniel Veillard wrote:
On Wed, Jul 28, 2010 at 05:34:14PM -0600, Eric Blake wrote:
> The recent switch to enable -Wlogical-op paid off again.
> gcc 4.5.0 (rawhide) is smarter than 4.4.4 (Fedora 13).
>
> * src/xen/xend_internal.c (xenDaemonAttachDeviceFlags)
> (xenDaemonUpdateDeviceFlags, xenDaemonDetachDeviceFlags): Use
> correct operator.
ACK,
Thanks; pushed.
I'll wait for Matthias' opinion on the ESX patch, since this was a bug
fix, but that one is just cosmetic.
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
6:34 p.m.
New subject: [libvirt] [PATCH 2/2] esx: silence spurious compiler warning
* src/esx/esx_vi_types.c (_DESERIALIZE_NUMBER)
(ESX_VI__TEMPLATE__DESERIALIZE_NUMBER): Add range check to shut up
gcc 4.5.0 regarding long long.
---
src/esx/esx_vi_types.c | 6 ++++--
1 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/src/esx/esx_vi_types.c b/src/esx/esx_vi_types.c
index 6e75995..5cf30b1 100644
--- a/src/esx/esx_vi_types.c
+++ b/src/esx/esx_vi_types.c
@@ -333,7 +333,8 @@
goto cleanup; \
} \
\
- if (value < (_min) || value > (_max)) { \
+ if (((_min) != INT64_MIN && value < (_min))
\
+ || ((_max) != INT64_MAX && value > (_max))) {
\
ESX_VI_ERROR(VIR_ERR_INTERNAL_ERROR, \
"Value '%s' is not representable as "_xsdType,
\
(const char *)string); \
@@ -922,7 +923,8 @@ esxVI_AnyType_Deserialize(xmlNodePtr node, esxVI_AnyType **anyType)
goto failure; \
} \
\
- if (number < (_min) || number > (_max)) { \
+ if (((_min) != INT64_MIN && number < (_min))
\
+ || ((_max) != INT64_MAX && number > (_max))) {
\
ESX_VI_ERROR(VIR_ERR_INTERNAL_ERROR, \
_("Value '%s' is out of %s range"),
\
(*anyType)->value, _xsdType); \
--
1.7.2
12:20 p.m.
New subject: [libvirt] [PATCH 2/2] esx: silence spurious compiler warning
On Wed, Jul 28, 2010 at 05:34:15PM -0600, Eric Blake wrote:
* src/esx/esx_vi_types.c (_DESERIALIZE_NUMBER)
(ESX_VI__TEMPLATE__DESERIALIZE_NUMBER): Add range check to shut up
gcc 4.5.0 regarding long long.
---
src/esx/esx_vi_types.c | 6 ++++--
1 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/src/esx/esx_vi_types.c b/src/esx/esx_vi_types.c
index 6e75995..5cf30b1 100644
--- a/src/esx/esx_vi_types.c
+++ b/src/esx/esx_vi_types.c
@@ -333,7 +333,8 @@
goto cleanup; \
} \
\
- if (value < (_min) || value > (_max)) { \
+ if (((_min) != INT64_MIN && value < (_min))
\
+ || ((_max) != INT64_MAX && value > (_max))) {
\
ESX_VI_ERROR(VIR_ERR_INTERNAL_ERROR, \
"Value '%s' is not representable as
"_xsdType, \
(const char *)string); \
@@ -922,7 +923,8 @@ esxVI_AnyType_Deserialize(xmlNodePtr node, esxVI_AnyType **anyType)
goto failure; \
} \
\
- if (number < (_min) || number > (_max)) { \
+ if (((_min) != INT64_MIN && number < (_min))
\
+ || ((_max) != INT64_MAX && number > (_max))) {
\
ESX_VI_ERROR(VIR_ERR_INTERNAL_ERROR, \
_("Value '%s' is out of %s range"),
\
(*anyType)->value, _xsdType); \
--
1.7.2
Way harder to understand than the original, IMHO
I defer to Matthias for this :-)
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
daniel(a)veillard.com | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
7:25 p.m.
New subject: [libvirt] [PATCH 2/2] esx: silence spurious compiler warning
2010/7/29 Eric Blake <eblake(a)redhat.com>:
* src/esx/esx_vi_types.c (_DESERIALIZE_NUMBER)
(ESX_VI__TEMPLATE__DESERIALIZE_NUMBER): Add range check to shut up
gcc 4.5.0 regarding long long.
---
src/esx/esx_vi_types.c | 6 ++++--
1 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/src/esx/esx_vi_types.c b/src/esx/esx_vi_types.c
index 6e75995..5cf30b1 100644
--- a/src/esx/esx_vi_types.c
+++ b/src/esx/esx_vi_types.c
@@ -333,7 +333,8 @@
goto cleanup; \
} \
\
- if (value < (_min) || value > (_max)) { \
+ if (((_min) != INT64_MIN && value < (_min))
\
+ || ((_max) != INT64_MAX && value > (_max))) {
\
ESX_VI_ERROR(VIR_ERR_INTERNAL_ERROR, \
"Value '%s' is not representable as "_xsdType,
\
(const char *)string); \
@@ -922,7 +923,8 @@ esxVI_AnyType_Deserialize(xmlNodePtr node, esxVI_AnyType **anyType)
goto failure; \
} \
\
- if (number < (_min) || number > (_max)) { \
+ if (((_min) != INT64_MIN && number < (_min))
\
+ || ((_max) != INT64_MAX && number > (_max))) {
\
ESX_VI_ERROR(VIR_ERR_INTERNAL_ERROR, \
_("Value '%s' is out of %s range"),
\
(*anyType)->value, _xsdType); \
--
1.7.2
It seems that newer gcc warns about this, because in case of _min/_max
== INT64_MIN/INT64_MAX the original condition is always false and
adding the additional checks avoids this situation.
ACK.
Matthias
7:35 a.m.
New subject: [libvirt] [PATCH 2/2] esx: silence spurious compiler warning
On 07/29/2010 06:25 PM, Matthias Bolte wrote:
2010/7/29 Eric Blake <eblake(a)redhat.com>:
> * src/esx/esx_vi_types.c (_DESERIALIZE_NUMBER)
> (ESX_VI__TEMPLATE__DESERIALIZE_NUMBER): Add range check to shut up
> gcc 4.5.0 regarding long long.
> ---
> src/esx/esx_vi_types.c | 6 ++++--
> 1 files changed, 4 insertions(+), 2 deletions(-)
> - if (value < (_min) || value > (_max)) {
\
> + if (((_min) != INT64_MIN && value < (_min))
\
> + || ((_max) != INT64_MAX && value > (_max))) {
\
>
It seems that newer gcc warns about this, because in case of _min/_max
== INT64_MIN/INT64_MAX the original condition is always false and
adding the additional checks avoids this situation.
The original condition is still false for INT64_MIN/INT64_MAX even with
the additional checks, but the additional checks are enough to shut up
gcc. Hopefully, since the additional checks are compile time constants
(based on all uses of the macros) it doesn't bloat the assembly any. I
just hope that we don't have to keep tweaking this down the road for
future gcc (the whole point of the macro is that we know we are using
the same code for multiple types, where the range checks are important
for all but one of the types).
ACK.
Thanks; pushed.
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
5230
days inactive
5232
days old
7 comments
3 participants
participants (3)
-
Daniel Veillard -
Eric Blake -
Matthias Bolte