[libvirt] [PATCH] virDomainFormatSchedDef: Initialize @priority
Older gcc fails to see that the variable is set iff @hasPriority == true in which case the former is set a value. Initialize the value while declaring it to make the compiler shut up. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- This maybe isn't the best approach, to workaround false positives. I'm open to discussion. src/conf/domain_conf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 895a51b..67415fa 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -21472,7 +21472,7 @@ virDomainFormatSchedDef(virDomainDefPtr def, virBitmapPtr currentMap = NULL; ssize_t nextprio; bool hasPriority = false; - int priority; + int priority = 0; switch ((virProcessSchedPolicy) i) { case VIR_PROC_POLICY_NONE: -- 2.4.10
On Tue, Feb 09, 2016 at 09:08:49 +0100, Michal Privoznik wrote:
Older gcc fails to see that the variable is set iff @hasPriority == true in which case the former is set a value. Initialize the value while declaring it to make the compiler shut up.
Initializing it may confuse a next reader, since there is no default value that would make sense at this point.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> ---
This maybe isn't the best approach, to workaround false positives. I'm open to discussion.
I'd suggest using non-broken compilers on non-ancient platforms and leaving the code be. Peter
On 09.02.2016 09:26, Peter Krempa wrote:
On Tue, Feb 09, 2016 at 09:08:49 +0100, Michal Privoznik wrote:
Older gcc fails to see that the variable is set iff @hasPriority == true in which case the former is set a value. Initialize the value while declaring it to make the compiler shut up.
Initializing it may confuse a next reader, since there is no default value that would make sense at this point.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> ---
This maybe isn't the best approach, to workaround false positives. I'm open to discussion.
I'd suggest using non-broken compilers on non-ancient platforms and leaving the code be.
Sometimes it's not possible. One can hardly expect a gcc upgrade on say RHEL-6 or CentOS-6. Question is, why would somebody want to compile libvirt from git on such stable systems, right? On the other hand, we tend to be build error free previously even on such systems. Michal
On Tue, Feb 09, 2016 at 09:29:48 +0100, Michal Privoznik wrote:
On 09.02.2016 09:26, Peter Krempa wrote:
On Tue, Feb 09, 2016 at 09:08:49 +0100, Michal Privoznik wrote:
---
This maybe isn't the best approach, to workaround false positives. I'm open to discussion.
I'd suggest using non-broken compilers on non-ancient platforms and leaving the code be.
Sometimes it's not possible. One can hardly expect a gcc upgrade on say RHEL-6 or CentOS-6. Question is, why would somebody want to compile libvirt from git on such stable systems, right? On the other hand, we tend to be build error free previously even on such systems.
You have to draw a line at some point. Your generic upstream libvirt won't just work with qemu on CentOS-6 since it was so heavily patched that the downstream libvirt carries a rather huge amount of patches that make them work together. This implies that it would require a upstream rebuild of qemu too. At that point you certainly don't get any of the "enterprise" reasons to use an enterprise as stability and testing and other stuff, so you might as well as upgrade anyways. Maybe it's time to finally stop fixing it to see whether people complain. Peter
On 09.02.2016 09:50, Peter Krempa wrote:
On Tue, Feb 09, 2016 at 09:29:48 +0100, Michal Privoznik wrote:
On 09.02.2016 09:26, Peter Krempa wrote:
On Tue, Feb 09, 2016 at 09:08:49 +0100, Michal Privoznik wrote:
---
This maybe isn't the best approach, to workaround false positives. I'm open to discussion.
I'd suggest using non-broken compilers on non-ancient platforms and leaving the code be.
Sometimes it's not possible. One can hardly expect a gcc upgrade on say RHEL-6 or CentOS-6. Question is, why would somebody want to compile libvirt from git on such stable systems, right? On the other hand, we tend to be build error free previously even on such systems.
You have to draw a line at some point. Your generic upstream libvirt won't just work with qemu on CentOS-6 since it was so heavily patched that the downstream libvirt carries a rather huge amount of patches that make them work together. This implies that it would require a upstream rebuild of qemu too. At that point you certainly don't get any of the "enterprise" reasons to use an enterprise as stability and testing and other stuff, so you might as well as upgrade anyways.
Maybe it's time to finally stop fixing it to see whether people complain.
Well, if that's what others people think as well, I'm good with that. We don't care for RHEL-5 any more and it seems like a good time to stop fixing RHEL-6/CentOS-6 too, doesn't it. Let me start a new thread because I expect not everybody to follow this thread. Michal
On Tue, Feb 09, 2016 at 09:26:33AM +0100, Peter Krempa wrote:
On Tue, Feb 09, 2016 at 09:08:49 +0100, Michal Privoznik wrote:
Older gcc fails to see that the variable is set iff @hasPriority == true in which case the former is set a value. Initialize the value while declaring it to make the compiler shut up.
Initializing it may confuse a next reader, since there is no default value that would make sense at this point.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> ---
This maybe isn't the best approach, to workaround false positives. I'm open to discussion.
I'd suggest using non-broken compilers on non-ancient platforms and leaving the code be.
GCC has a long history of both giving false warnings about uninitialized variables, and missing warnings about genuinely uninitialized vars, not to mention bugs in various other warning flags. As such it is perfectly reasonable to add stuff to the code to shutup bogus compiler warnings, particularly when it is as simple as initializing a variable. We've done it many times in the past already. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On Tue, Feb 09, 2016 at 09:08:49AM +0100, Michal Privoznik wrote:
Older gcc fails to see that the variable is set iff @hasPriority == true in which case the former is set a value. Initialize the value while declaring it to make the compiler shut up.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> ---
This maybe isn't the best approach, to workaround false positives. I'm open to discussion.
I think it is fine as is. Even if gcc were no reporting warnings, it is always valid to initialize variables to some default value. It may currently be redundant, but code is often refactored / extended later, at which point the default value may well protect against a bug, because compilers are not perfect at finding use of uninitialized variables.
src/conf/domain_conf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 895a51b..67415fa 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -21472,7 +21472,7 @@ virDomainFormatSchedDef(virDomainDefPtr def, virBitmapPtr currentMap = NULL; ssize_t nextprio; bool hasPriority = false; - int priority; + int priority = 0;
switch ((virProcessSchedPolicy) i) { case VIR_PROC_POLICY_NONE:
ACK Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
participants (3)
-
Daniel P. Berrange -
Michal Privoznik -
Peter Krempa