Re: [PATCH 3/7] src: remove fallback when opening dnsmasq addnhosts file fails

[Laine Stump via Devel]

On 10/26/26 8:18 AM, Daniel P. Berrangé via Devel wrote:
From: Daniel P. Berrangé <berra...@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é <berra...@redhat.com>

Reviewed-by: Laine Stump <la...@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;