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

[Matthias Bolte <matthias bolte googlemail com>]

2011/7/5 Eric Blake <eblake redhat com>:
> 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.

You're right. Here's a v2, I missed to post it as a proper v2 to this thread

https://www.redhat.com/archives/libvir-list/2011-July/msg00205.html

-- 
Matthias Bolte
http://photron.blogspot.com