xml_pretty_print_in_place: close file handles and allocations correctly

1 files changed, 21 insertions, 24 deletions

diff --git a/src/lib/core/utils.c b/src/lib/core/utils.c
index 8e48df3..047fa16 100644
--- a/src/lib/core/utils.c
+++ b/src/lib/core/utils.c
@@ -609,10 +609,7 @@ int path_store(char **destptr, size_t maxlen, const char *base, const char *path
 
 int xml_pretty_print_in_place(const char *filename, const char *pretty_print_prog, const char *pretty_print_args) {
     int status = 0;
-    char *tempfile = NULL;
     char *result = NULL;
-    FILE *fp = NULL;
-    FILE *tmpfp = NULL;
     char cmd[PATH_MAX];
     if (!find_program(pretty_print_prog)) {
         // Pretty printing is optional. 99% chance the XML data will
@@ -623,46 +620,46 @@ int xml_pretty_print_in_place(const char *filename, const char *pretty_print_pro
     snprintf(cmd, sizeof(cmd), "%s %s %s", pretty_print_prog, pretty_print_args, filename);
     result = shell_output(cmd, &status);
     if (status || !result) {
-        goto pretty_print_failed;
+        return status;
     }
 
-    tempfile = xmkstemp(&tmpfp, "w+");
-    if (!tmpfp || !tempfile) {
+    int clean_up_fp = 0;
+    FILE *tmpfp = NULL;
+    char *tempfile = xmkstemp(&tmpfp, "w+");
+    if (!tempfile || !tmpfp) {
+        guard_free(tempfile);
+        if (tmpfp) {
+            fclose(tmpfp);
+        }
+        status = -1;
         goto pretty_print_failed;
     }
-
     fprintf(tmpfp, "%s", result);
     fflush(tmpfp);
     fclose(tmpfp);
 
-    fp = fopen(filename, "w+");
+    FILE *fp = fopen(filename, "w+");
     if (!fp) {
+        status = -1;
         goto pretty_print_failed;
     }
+    clean_up_fp = 1;
 
     if (copy2(tempfile, filename, CT_PERM)) {
-        goto pretty_print_failed;
+        SYSERROR("%s", "copy operation failed");
     }
 
-    if (remove(tempfile)) {
-        goto pretty_print_failed;
+    pretty_print_failed:
+    if (tempfile && remove(tempfile)) {
+        SYSERROR("unable to remove temporary file: %s", tempfile);
     }
-
-    fclose(fp);
     guard_free(tempfile);
     guard_free(result);
-    return 0;
+    if (clean_up_fp) {
+        fclose(fp);
+    }
 
-    pretty_print_failed:
-        if (fp) {
-            fclose(fp);
-        }
-        if (tmpfp) {
-            fclose(tmpfp);
-        }
-        guard_free(tempfile);
-        guard_free(result);
-        return -1;
+    return status;
 }
 
 /**