On 10/26/26 8:18 AM, Daniel P. Berrangé via Devel wrote:
From: Daniel P. Berrangé <berrange@redhat.com>
When writing the dnsmasq addnhosts file to a temporary file, there is a fallback which tries to open the original file. We expect creating the temporary file to always succeed, and if it fails something serious has gone wrong that we want to have reported. So this fallback is expected to be dead code.
I guess the point of writing to the temp file is either 1) so that the original is still there in the case that writing the new one fails, or 2) to make the change atomic? This code has been around so long that I only barely recognize the original author's name, much less have any idea why it is the way it is :-P (admittedly I didn't spend much time thinking about it). But anyway if it's being done, there must have been some reason for it, and I can't think of any good reason why we would fail to open and write to "x.tmp" but then succeed to write to "x".
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com>
--- src/util/virdnsmasq.c | 25 ++++++------------------- 1 file changed, 6 insertions(+), 19 deletions(-)
diff --git a/src/util/virdnsmasq.c b/src/util/virdnsmasq.c index 7d6de943cf..817e060a44 100644 --- a/src/util/virdnsmasq.c +++ b/src/util/virdnsmasq.c @@ -161,7 +161,6 @@ addnhostsWrite(const char *path, { g_autofree char *tmp = NULL; FILE *f; - bool istmp = true; size_t i, j; int rc = 0;
@@ -171,20 +170,14 @@ addnhostsWrite(const char *path,
tmp = g_strdup_printf("%s.new", path);
- if (!(f = fopen(tmp, "w"))) { - istmp = false; - if (!(f = fopen(path, "w"))) - return -errno; - } + if (!(f = fopen(tmp, "w"))) + return -errno;
for (i = 0; i < nhosts; i++) { if (fputs(hosts[i].ip, f) == EOF || fputc('\t', f) == EOF) { rc = -errno; VIR_FORCE_FCLOSE(f); - - if (istmp) - unlink(tmp); - + unlink(tmp); return rc; }
@@ -192,10 +185,7 @@ addnhostsWrite(const char *path, if (fputs(hosts[i].hostnames[j], f) == EOF || fputc('\t', f) == EOF) { rc = -errno; VIR_FORCE_FCLOSE(f); - - if (istmp) - unlink(tmp); - + unlink(tmp); return rc; } } @@ -203,10 +193,7 @@ addnhostsWrite(const char *path, if (fputc('\n', f) == EOF) { rc = -errno; VIR_FORCE_FCLOSE(f); - - if (istmp) - unlink(tmp); - + unlink(tmp); return rc; } } @@ -214,7 +201,7 @@ addnhostsWrite(const char *path, if (VIR_FCLOSE(f) == EOF) return -errno;
- if (istmp && rename(tmp, path) < 0) { + if (rename(tmp, path) < 0) { rc = -errno; unlink(tmp); return rc;