On Mon, Jul 19, 2010 at 09:21:55PM -0400, Laine Stump wrote:
One error exit in virStorageBackendCreateBlockFrom was setting the return value to errno. The convention for volume build functions is to return 0 on success or -1 on failure. Not only was it not necessary to set the return value (it defaults to -1, and is set to 0 when everything has been successfully completed), in the case that some caller were checking for < 0 rather than != 0, they would incorrectly believe that it completed successfully. --- src/storage/storage_backend.c | 1 - 1 files changed, 0 insertions(+), 1 deletions(-)
diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index 5b61bba..d989743 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -232,7 +232,6 @@ virStorageBackendCreateBlockFrom(virConnectPtr conn ATTRIBUTE_UNUSED, }
if (fstat(fd, &st) == -1) { - ret = errno; virReportSystemError(errno, _("stat of '%s' failed"), vol->target.path); goto cleanup;
ACK, but it seems to me ret being initialized to -1, maybe it's better to be consistent and on all error do a ret = -errno; before virReportSystemError and the goto cleanup. Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/