On 07/05/2011 08:00 AM, Matthias Bolte wrote:
Also be explicity about the != 0 check in the few places that weren't.
s/explicity/explicit/
--- src/conf/domain_conf.c | 6 +++--- src/conf/network_conf.c | 2 +- src/conf/nwfilter_conf.c | 4 ++-- src/conf/storage_conf.c | 2 +- src/libxl/libxl_driver.c | 20 ++++++++++---------- src/lxc/lxc_container.c | 16 ++++++++-------- src/lxc/lxc_controller.c | 6 +++--- src/lxc/lxc_driver.c | 2 +- src/network/bridge_driver.c | 6 +++--- src/qemu/qemu_driver.c | 28 ++++++++++++++-------------- src/qemu/qemu_process.c | 6 +++--- src/storage/storage_driver.c | 2 +- src/uml/uml_driver.c | 13 +++++++------ src/util/dnsmasq.c | 2 +- src/util/util.c | 2 +- 15 files changed, 59 insertions(+), 58 deletions(-)
As long as we are already auditing all callers,...
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 109a947..2467fcf 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -9980,14 +9980,14 @@ int virDomainSaveXML(const char *configDir, const char *xml) { char *configFile = NULL; - int fd = -1, ret = -1; + int fd = -1, ret = -1, err; size_t towrite;
if ((configFile = virDomainConfigFile(configDir, def->name)) == NULL) goto cleanup;
- if (virFileMakePath(configDir)) { - virReportSystemError(errno, + if ((err = virFileMakePath(configDir)) != 0) { + virReportSystemError(err,
I'm half inclined to NACK this patch, and instead request that we fix virFileMakePath to be more like the bulk of our code: return 0 on success, -1 on error, and guarantee that errno is valid on error. Then we can simply write: if (virFileMakePath(configDir) < 0) { virReportSystemError(errno, ... without needing an extra 'err' variable.
+++ b/src/util/util.c @@ -1182,7 +1182,7 @@ int virFileWritePid(const char *dir, goto cleanup; }
- if ((rc = virFileMakePath(dir))) + if ((rc = virFileMakePath(dir)) != 0)
The biggest problem with this patch is that virFileMakePath still has no documentation. I guess I could be okay with the approach in this patch if it were to also include a comment just before virFileMakePath documenting that on error the return is a positive(!) errno value, if that means that fewer callers have to change, although I still feel that going with our normal conventions of a negative return on error would be a better cleanup. At any rate, until virFileMakePath is documented, we will continue to have problems with clients using it incorrectly. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org