[libvirt] [PATCH] More coverity findings addressed
More bug extermination in the category of: Error: CHECKED_RETURN: /libvirt/src/conf/network_conf.c:595: check_return: Calling function "virAsprintf" without checking return value (as is done elsewhere 515 out of 543 times). /libvirt/src/qemu/qemu_process.c:2780: unchecked_value: No check of the return value of "virAsprintf(&msg, "was paused (%s)", virDomainPausedReasonTypeToString(reason))". /libvirt/tests/commandtest.c:809: check_return: Calling function "setsid" without checking return value (as is done elsewhere 4 out of 5 times). /libvirt/tests/commandtest.c:830: unchecked_value: No check of the return value of "virTestGetDebug()". /libvirt/tests/commandtest.c:831: check_return: Calling function "virTestGetVerbose" without checking return value (as is done elsewhere 41 out of 42 times). /libvirt/tests/commandtest.c:833: check_return: Calling function "virInitialize" without checking return value (as is done elsewhere 18 out of 21 times). One note about the error in commandtest line 809: setsid() seems to fail when running the test -- could be removed ? --- src/conf/network_conf.c | 7 ++----- src/qemu/qemu_process.c | 4 ++-- tests/commandtest.c | 9 +++++---- 3 files changed, 9 insertions(+), 11 deletions(-) Index: libvirt-acl/src/conf/network_conf.c =================================================================== --- libvirt-acl.orig/src/conf/network_conf.c +++ libvirt-acl/src/conf/network_conf.c @@ -590,12 +590,9 @@ virNetworkDNSSrvDefParseXML(virNetworkDN } if (strlen(service) > DNS_RECORD_LENGTH_SRV) { - char *name = NULL; - - virAsprintf(&name, _("Service name is too long, limit is %d bytes"), DNS_RECORD_LENGTH_SRV); virNetworkReportError(VIR_ERR_XML_DETAIL, - "%s", name); - VIR_FREE(name); + _("Service name is too long, limit is %d bytes"), + DNS_RECORD_LENGTH_SRV); goto error; } Index: libvirt-acl/src/qemu/qemu_process.c =================================================================== --- libvirt-acl.orig/src/qemu/qemu_process.c +++ libvirt-acl/src/qemu/qemu_process.c @@ -2777,8 +2777,8 @@ qemuProcessUpdateState(struct qemud_driv } else { newState = VIR_DOMAIN_PAUSED; newReason = reason; - virAsprintf(&msg, "was paused (%s)", - virDomainPausedReasonTypeToString(reason)); + ignore_value(virAsprintf(&msg, "was paused (%s)", + virDomainPausedReasonTypeToString(reason))); } } else if (state == VIR_DOMAIN_SHUTOFF && running) { newState = VIR_DOMAIN_RUNNING; Index: libvirt-acl/tests/commandtest.c =================================================================== --- libvirt-acl.orig/tests/commandtest.c +++ libvirt-acl/tests/commandtest.c @@ -806,7 +806,7 @@ mymain(void) return EXIT_FAILURE; setpgid(0, 0); - setsid(); + ignore_value(setsid()); /* Our test expects particular fd values; to get that, we must not * leak fds that we inherited from a lazy parent. At the same @@ -827,10 +827,11 @@ mymain(void) /* Prime the debug/verbose settings from the env vars, * since we're about to reset 'environ' */ - virTestGetDebug(); - virTestGetVerbose(); + ignore_value(virTestGetDebug()); + ignore_value(virTestGetVerbose()); - virInitialize(); + if (virInitialize() < 0) + return EXIT_FAILURE; /* Phase two of killing interfering fds; see above. */ fd = 3;
On 04/27/2012 12:34 PM, Stefan Berger wrote:
More bug extermination in the category of:
Error: CHECKED_RETURN:
One note about the error in commandtest line 809: setsid() seems to fail when running the test -- could be removed ?
Odd. I think we had to add it to make the test pass under some situations, so I don't think we can remove it, but I'd have to research why it is failing in other situations. It may stem from the history of whether the test is being run from another session leader process, and that may differ according to whether make is running the test or you are doing it by hand.
if (strlen(service) > DNS_RECORD_LENGTH_SRV) { - char *name = NULL; - - virAsprintf(&name, _("Service name is too long, limit is %d bytes"), DNS_RECORD_LENGTH_SRV); virNetworkReportError(VIR_ERR_XML_DETAIL, - "%s", name); - VIR_FREE(name); + _("Service name is too long, limit is %d bytes"), + DNS_RECORD_LENGTH_SRV);
Nice removal of an extra malloc.
goto error; }
Index: libvirt-acl/src/qemu/qemu_process.c =================================================================== --- libvirt-acl.orig/src/qemu/qemu_process.c +++ libvirt-acl/src/qemu/qemu_process.c @@ -2777,8 +2777,8 @@ qemuProcessUpdateState(struct qemud_driv } else { newState = VIR_DOMAIN_PAUSED; newReason = reason; - virAsprintf(&msg, "was paused (%s)", - virDomainPausedReasonTypeToString(reason)); + ignore_value(virAsprintf(&msg, "was paused (%s)", + virDomainPausedReasonTypeToString(reason)));
Safe, since we check msg for NULL later.
+++ libvirt-acl/tests/commandtest.c @@ -806,7 +806,7 @@ mymain(void) return EXIT_FAILURE;
setpgid(0, 0); - setsid(); + ignore_value(setsid());
Maybe worth logging a warning if we see the failure, but since this is test code, I'm also okay if we just ignore it (as long as the test still passes, of course). ACK. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 04/27/2012 03:17 PM, Eric Blake wrote:
On 04/27/2012 12:34 PM, Stefan Berger wrote:
More bug extermination in the category of:
Error: CHECKED_RETURN:
Maybe worth logging a warning if we see the failure, but since this is test code, I'm also okay if we just ignore it (as long as the test still passes, of course).
ACK.
Pushed (as posted)
On 04/27/2012 03:17 PM, Eric Blake wrote:
One note about the error in commandtest line 809: setsid() seems to fail when running the test -- could be removed ? Odd. I think we had to add it to make the test pass under some situations, so I don't think we can remove it, but I'd have to research why it is failing in other situations. It may stem from the history of whether the test is being run from another session leader process, and
On 04/27/2012 12:34 PM, Stefan Berger wrote: that may differ according to whether make is running the test or you are doing it by hand.
Right. the man page says that setsid will fail if the process is already the session leader. It says that in order to ensure success, you should fork, then exit the parent and call setsid in the child. I haven't looked at the code at all to see what circumstances would cause it to behave differently.
participants (3)
-
Eric Blake -
Laine Stump -
Stefan Berger