Re: [libvirt] [PATCH] Fix usage of virFileMakePath, it doesn't set errno but returns it

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.