From d36940992ed81053fdbf8117ce6da30a8fef2a0f Mon Sep 17 00:00:00 2001
From: Pino Toscano <ptoscano@redhat.com>
Date: Mon, 6 Mar 2017 14:23:55 +0100
Subject: [PATCH] lib: qemu: improve handling of FILE*

Create own blocks for all the parts dealing with FILE*: this way there
is no need to recycle the same FILE* variable for all the operations,
and have each block its own variable automatically cleaned up.

This also fixes a potential undefined behaviour on error: POSIX says
that after a call fclose(), a FILE* cannot be used anymore, not even
on fclose() failure. The previous behaviour for fclose == -1 was to jump
to the error label, which would then try to call fclose() again (since
the FILE* pointer was still non-null).
---
 lib/qemu.c | 104 ++++++++++++++++++++++++++---------------------------
 1 file changed, 52 insertions(+), 52 deletions(-)

diff --git a/lib/qemu.c b/lib/qemu.c
index e61bc3c44..d60692f0d 100644
--- a/lib/qemu.c
+++ b/lib/qemu.c
@@ -80,7 +80,6 @@ guestfs_int_test_qemu (guestfs_h *g, struct version *qemu_version)
   struct stat statbuf;
   CLEANUP_FREE char *cachedir = NULL, *qemu_stat_filename = NULL,
     *qemu_help_filename = NULL, *qemu_devices_filename = NULL;
-  FILE *fp;
   int generation;
   uint64_t prev_size, prev_mtime;
 
@@ -101,15 +100,16 @@ guestfs_int_test_qemu (guestfs_h *g, struct version *qemu_version)
   debug (g, "checking for previously cached test results of %s, in %s",
          g->hv, cachedir);
 
-  fp = fopen (qemu_stat_filename, "r");
-  if (fp == NULL)
-    goto do_test;
-  if (fscanf (fp, "%d %" SCNu64 " %" SCNu64,
-              &generation, &prev_size, &prev_mtime) != 3) {
-    fclose (fp);
-    goto do_test;
+  {
+    CLEANUP_FCLOSE FILE *fp = NULL;
+    fp = fopen (qemu_stat_filename, "r");
+    if (fp == NULL)
+      goto do_test;
+    if (fscanf (fp, "%d %" SCNu64 " %" SCNu64,
+                &generation, &prev_size, &prev_mtime) != 3) {
+      goto do_test;
+    }
   }
-  fclose (fp);
 
   if (generation == MEMO_GENERATION &&
       (uint64_t) statbuf.st_size == prev_size &&
@@ -153,54 +153,54 @@ guestfs_int_test_qemu (guestfs_h *g, struct version *qemu_version)
   /* Now memoize the qemu output in the cache directory. */
   debug (g, "saving test results");
 
-  fp = fopen (qemu_help_filename, "w");
-  if (fp == NULL) {
-  help_error:
-    perrorf (g, "%s", qemu_help_filename);
-    if (fp != NULL) fclose (fp);
-    guestfs_int_free_qemu_data (data);
-    return NULL;
+  {
+    CLEANUP_FCLOSE FILE *fp = NULL;
+    fp = fopen (qemu_help_filename, "w");
+    if (fp == NULL) {
+    help_error:
+      perrorf (g, "%s", qemu_help_filename);
+      guestfs_int_free_qemu_data (data);
+      return NULL;
+    }
+    if (fprintf (fp, "%s", data->qemu_help) == -1)
+      goto help_error;
   }
-  if (fprintf (fp, "%s", data->qemu_help) == -1)
-    goto help_error;
-  if (fclose (fp) == -1)
-    goto help_error;
 
-  fp = fopen (qemu_devices_filename, "w");
-  if (fp == NULL) {
-  devices_error:
-    perrorf (g, "%s", qemu_devices_filename);
-    if (fp != NULL) fclose (fp);
-    guestfs_int_free_qemu_data (data);
-    return NULL;
+  {
+    CLEANUP_FCLOSE FILE *fp = NULL;
+    fp = fopen (qemu_devices_filename, "w");
+    if (fp == NULL) {
+    devices_error:
+      perrorf (g, "%s", qemu_devices_filename);
+      guestfs_int_free_qemu_data (data);
+      return NULL;
+    }
+    if (fprintf (fp, "%s", data->qemu_devices) == -1)
+      goto devices_error;
   }
-  if (fprintf (fp, "%s", data->qemu_devices) == -1)
-    goto devices_error;
-  if (fclose (fp) == -1)
-    goto devices_error;
 
-  /* Write the qemu.stat file last so that its presence indicates that
-   * the qemu.help and qemu.devices files ought to exist.
-   */
-  fp = fopen (qemu_stat_filename, "w");
-  if (fp == NULL) {
-  stat_error:
-    perrorf (g, "%s", qemu_stat_filename);
-    if (fp != NULL) fclose (fp);
-    guestfs_int_free_qemu_data (data);
-    return NULL;
+  {
+    /* Write the qemu.stat file last so that its presence indicates that
+     * the qemu.help and qemu.devices files ought to exist.
+     */
+    CLEANUP_FCLOSE FILE *fp = NULL;
+    fp = fopen (qemu_stat_filename, "w");
+    if (fp == NULL) {
+    stat_error:
+      perrorf (g, "%s", qemu_stat_filename);
+      guestfs_int_free_qemu_data (data);
+      return NULL;
+    }
+    /* The path to qemu is stored for information only, it is not
+     * used when we parse the file.
+     */
+    if (fprintf (fp, "%d %" PRIu64 " %" PRIu64 " %s\n",
+                 MEMO_GENERATION,
+                 (uint64_t) statbuf.st_size,
+                 (uint64_t) statbuf.st_mtime,
+                 g->hv) == -1)
+      goto stat_error;
   }
-  /* The path to qemu is stored for information only, it is not
-   * used when we parse the file.
-   */
-  if (fprintf (fp, "%d %" PRIu64 " %" PRIu64 " %s\n",
-               MEMO_GENERATION,
-               (uint64_t) statbuf.st_size,
-               (uint64_t) statbuf.st_mtime,
-               g->hv) == -1)
-    goto stat_error;
-  if (fclose (fp) == -1)
-    goto stat_error;
 
   return data;
 }