Re: [PATCH 4/7] src: use virFileRewrite for dnsmasq addnhosts file

30 Oct 2026

On 10/26/26 8:18 AM, Daniel P. Berrangé via Devel wrote:
...
From: Daniel P. Berrangé <berrange@redhat.com>
The current addnhostsWrite() method implements logic that is
already present in virFileRewrite.
...and then some - it also does an fsync() before closing the new file, 
I guess to make sure that the new file really truly is all on disk and 
ready to use by the time the rename happens.

Am I correct in thinking you included patch 3 (and also upcoming patch 
5) just to make it more obvious that this was just doing a 
straight-across replacement of functionality? Because otherwise it would 
totally be okay with me to squash 3&4 together, and also 5&6.

BTW, I also noticed that these patches remove the only uses of fputs() 
that gets compiled into any daemons (all that's left is a couple of 
tests, and a few uses in tools/vsh.c). Is fputs() one of those 
problematic (not async-signal safe) functions that we should ban via a 
syntax-check rule?
...
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com>
...
---
  src/util/virdnsmasq.c | 82 ++++++++++++++++---------------------------
  1 file changed, 31 insertions(+), 51 deletions(-)

diff --git a/src/util/virdnsmasq.c b/src/util/virdnsmasq.c
index 817e060a44..656665e174 100644
--- a/src/util/virdnsmasq.c
+++ b/src/util/virdnsmasq.c
@@ -154,77 +154,57 @@ addnhostsNew(const char *name,
      return NULL;
  }
+
  static int
-addnhostsWrite(const char *path,
-               dnsmasqAddnHost *hosts,
-               unsigned int nhosts)
+addnhostsWrite(int fd,
+               const char *path,
+               const void *opaque)
  {
-    g_autofree char *tmp = NULL;
-    FILE *f;
+    const dnsmasqAddnHostsfile *addnhostsfile = opaque;
      size_t i, j;
-    int rc = 0;
/* even if there are 0 hosts, create a 0 length file, to allow
       * for runtime addition.
       */
-    tmp = g_strdup_printf("%s.new", path);
-
-    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);
-            unlink(tmp);
-            return rc;
-        }
-
-        for (j = 0; j < hosts[i].nhostnames; j++) {
-            if (fputs(hosts[i].hostnames[j], f) == EOF || fputc('\t', f) == EOF) {
-                rc = -errno;
-                VIR_FORCE_FCLOSE(f);
-                unlink(tmp);
-                return rc;
-            }
-        }
-
-        if (fputc('\n', f) == EOF) {
-            rc = -errno;
-            VIR_FORCE_FCLOSE(f);
-            unlink(tmp);
-            return rc;
+    for (i = 0; i < addnhostsfile->nhosts; i++) {
+        if (safewrite(fd, addnhostsfile->hosts[i].ip,
+                      strlen(addnhostsfile->hosts[i].ip)) < 0)
+            goto error;
+        if (safewrite(fd, "\t", 1) < 0)
+            goto error;
+
+        for (j = 0; j < addnhostsfile->hosts[i].nhostnames; j++) {
+            if (safewrite(fd, addnhostsfile->hosts[i].hostnames[j],
+                          strlen(addnhostsfile->hosts[i].hostnames[j])) < 0)
+                goto error;
+            if (safewrite(fd, "\t", 1) < 0)
+                goto error;
          }
-    }
-
-    if (VIR_FCLOSE(f) == EOF)
-        return -errno;
-    if (rename(tmp, path) < 0) {
-        rc = -errno;
-        unlink(tmp);
-        return rc;
+        if (safewrite(fd, "\n", 1) < 0)
+            goto error;
      }
return 0;
+
+ error:
+    virReportSystemError(errno,
+                         _("cannot write config file '%1$s'"),
+                         path);
+    return -1;
  }
static int
  addnhostsSave(dnsmasqAddnHostsfile *addnhostsfile)
  {
-    int err = addnhostsWrite(addnhostsfile->path, addnhostsfile->hosts,
-                             addnhostsfile->nhosts);
-
-    if (err < 0) {
-        virReportSystemError(-err, _("cannot write config file '%1$s'"),
-                             addnhostsfile->path);
-        return -1;
-    }
-
-    return 0;
+    return virFileRewrite(addnhostsfile->path,
+                          0600, -1, -1,
+                          addnhostsWrite,
+                          addnhostsfile);
  }
+
  static int
  genericFileDelete(char *path)
  {

    