[libvirt] [PATCH] dnsmasq: Fix errno handling and don't unlink non-existing files

[Matthias Bolte]

2011/6/29 Daniel Veillard <veillard at redhat.com>:
> On Wed, Jun 29, 2011 at 02:14:19AM +0200, Matthias Bolte wrote:
>> addnhostsSave and hostsfileSave expect < 0 return value on error from
>> addnhostsWrite and hostsfileWrite but then pass err instead of -err
>> to virReportSystemError that expects an errno value.
>>
>> Also addnhostsWrite returns -ENOMEM and errno, change this to -errno.
>>
>> addnhostsWrite and hostsfileWrite tried to unlink the tempfile after
>> renaming it, making both fail on the final step. Remove the unnecessary
>> unlink calls.
>> ---
>>  src/util/dnsmasq.c |   44 +++++++++++++++-----------------------------
>>  1 files changed, 15 insertions(+), 29 deletions(-)
>>
>> diff --git a/src/util/dnsmasq.c b/src/util/dnsmasq.c
>> index 5baa34f..aadca10 100644
>> --- a/src/util/dnsmasq.c
>> +++ b/src/util/dnsmasq.c
>> @@ -185,14 +185,14 @@ addnhostsWrite(const char *path,
>>      if (!(f = fopen(tmp, "w"))) {
>>          istmp = false;
>>          if (!(f = fopen(path, "w"))) {
>> -            rc = errno;
>> +            rc = -errno;
>>              goto cleanup;
>>          }
>>      }
>>
>>      for (i = 0; i < nhosts; i++) {
>>          if (fputs(hosts[i].ip, f) == EOF || fputc('\t', f) == EOF) {
>> -            rc = errno;
>> +            rc = -errno;
>>              VIR_FORCE_FCLOSE(f);
>>
>>              if (istmp)
>> @@ -203,7 +203,7 @@ addnhostsWrite(const char *path,
>>
>>          for (ii = 0; ii < hosts[i].nhostnames; ii++) {
>>              if (fputs(hosts[i].hostnames[ii], f) == EOF || fputc('\t', f) == EOF) {
>> -                rc = errno;
>> +                rc = -errno;
>>                  VIR_FORCE_FCLOSE(f);
>>
>>                  if (istmp)
>> @@ -214,7 +214,7 @@ addnhostsWrite(const char *path,
>>          }
>>
>>          if (fputc('\n', f) == EOF) {
>> -            rc = errno;
>> +            rc = -errno;
>>              VIR_FORCE_FCLOSE(f);
>>
>>              if (istmp)
>> @@ -225,21 +225,14 @@ addnhostsWrite(const char *path,
>>      }
>>
>>      if (VIR_FCLOSE(f) == EOF) {
>> -        rc = errno;
>> +        rc = -errno;
>>          goto cleanup;
>>      }
>>
>> -    if (istmp) {
>> -        if (rename(tmp, path) < 0) {
>> -            rc = errno;
>> -            unlink(tmp);
>> -            goto cleanup;
>> -        }
>> -
>> -        if (unlink(tmp) < 0) {
>> -            rc = errno;
>> -            goto cleanup;
>> -        }
>> +    if (istmp && rename(tmp, path) < 0) {
>> +        rc = -errno;
>> +        unlink(tmp);
>> +        goto cleanup;
>>      }
>>
>>   cleanup:
>> @@ -255,7 +248,7 @@ addnhostsSave(dnsmasqAddnHostsfile *addnhostsfile)
>>                               addnhostsfile->nhosts);
>>
>>      if (err < 0) {
>> -        virReportSystemError(err, _("cannot write config file '%s'"),
>> +        virReportSystemError(-err, _("cannot write config file '%s'"),
>>                               addnhostsfile->path);
>>          return -1;
>>      }
>> @@ -402,17 +395,10 @@ hostsfileWrite(const char *path,
>>          goto cleanup;
>>      }
>>
>> -    if (istmp) {
>> -        if (rename(tmp, path) < 0) {
>> -            rc = -errno;
>> -            unlink(tmp);
>> -            goto cleanup;
>> -        }
>> -
>> -        if (unlink(tmp) < 0) {
>> -            rc = -errno;
>> -            goto cleanup;
>> -        }
>> +    if (istmp && rename(tmp, path) < 0) {
>> +        rc = -errno;
>> +        unlink(tmp);
>> +        goto cleanup;
>>      }
>>
>>   cleanup:
>> @@ -428,7 +414,7 @@ hostsfileSave(dnsmasqHostsfile *hostsfile)
>>                               hostsfile->nhosts);
>>
>>      if (err < 0) {
>> -        virReportSystemError(err, _("cannot write config file '%s'"),
>> +        virReportSystemError(-err, _("cannot write config file '%s'"),
>>                               hostsfile->path);
>>          return -1;
>>      }
>
>  ACK,
>
> Daniel
>

Thanks, pushed.

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