[libvirt] [PATCH] dnsmasq: Fix errno handling and don't unlink non-existing files
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; } -- 1.7.0.4
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 -- 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/
2011/6/29 Daniel Veillard <veillard@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
participants (2)
-
Daniel Veillard -
Matthias Bolte