difftool: address a couple of resource/memory leaks

This change plugs a couple of memory leaks and makes sure that the file
descriptor is closed in run_dir_diff().

Spotted by Coverity.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/builtin/difftool.c b/builtin/difftool.c
index 1354d0e4625c855676e7812be92d7c77bdecb407..b9a892f2693efe01a08c958f7064f13ba6ec43aa 100644 (file)
--- a/builtin/difftool.c
+++ b/builtin/difftool.c
@@ -226,6 +226,7 @@ static void changed_files(struct hashmap *result, const char *index_path,
                hashmap_entry_init(entry, strhash(buf.buf));
                hashmap_add(result, entry);
        }
                hashmap_entry_init(entry, strhash(buf.buf));
                hashmap_add(result, entry);
        }

+       fclose(fp);

        if (finish_command(&diff_files))
                die("diff-files did not exit properly");
        strbuf_release(&index_env);
        if (finish_command(&diff_files))
                die("diff-files did not exit properly");
        strbuf_release(&index_env);


@@ -439,8 +440,10 @@ static int run_dir_diff(const char *extcmd, int symlinks, const char *prefix,
                }
 
                if (lmode && status != 'C') {
                }
 
                if (lmode && status != 'C') {

-                       if (checkout_path(lmode, &loid, src_path, &lstate))
-                               return error("could not write '%s'", src_path);
+                       if (checkout_path(lmode, &loid, src_path, &lstate)) {
+                               ret = error("could not write '%s'", src_path);
+                               goto finish;
+                       }

                }
 
                if (rmode && !S_ISLNK(rmode)) {
                }
 
                if (rmode && !S_ISLNK(rmode)) {


@@ -456,9 +459,12 @@ static int run_dir_diff(const char *extcmd, int symlinks, const char *prefix,
                        hashmap_add(&working_tree_dups, entry);
 
                        if (!use_wt_file(workdir, dst_path, &roid)) {
                        hashmap_add(&working_tree_dups, entry);
 
                        if (!use_wt_file(workdir, dst_path, &roid)) {

-                               if (checkout_path(rmode, &roid, dst_path, &rstate))
-                                       return error("could not write '%s'",
-                                                    dst_path);
+                               if (checkout_path(rmode, &roid, dst_path,
+                                                 &rstate)) {
+                                       ret = error("could not write '%s'",
+                                                   dst_path);
+                                       goto finish;
+                               }

                        } else if (!is_null_oid(&roid)) {
                                /*
                                 * Changes in the working tree need special
                        } else if (!is_null_oid(&roid)) {
                                /*
                                 * Changes in the working tree need special


@@ -473,10 +479,12 @@ static int run_dir_diff(const char *extcmd, int symlinks, const char *prefix,
                                                ADD_CACHE_JUST_APPEND);
 
                                add_path(&rdir, rdir_len, dst_path);
                                                ADD_CACHE_JUST_APPEND);
 
                                add_path(&rdir, rdir_len, dst_path);

-                               if (ensure_leading_directories(rdir.buf))
-                                       return error("could not create "
-                                                    "directory for '%s'",
-                                                    dst_path);
+                               if (ensure_leading_directories(rdir.buf)) {
+                                       ret = error("could not create "
+                                                   "directory for '%s'",
+                                                   dst_path);
+                                       goto finish;
+                               }

                                add_path(&wtdir, wtdir_len, dst_path);
                                if (symlinks) {
                                        if (symlink(wtdir.buf, rdir.buf)) {
                                add_path(&wtdir, wtdir_len, dst_path);
                                if (symlinks) {
                                        if (symlink(wtdir.buf, rdir.buf)) {


@@ -497,13 +505,15 @@ static int run_dir_diff(const char *extcmd, int symlinks, const char *prefix,
                }
        }
 
                }
        }
 

+       fclose(fp);
+       fp = NULL;

        if (finish_command(&child)) {
                ret = error("error occurred running diff --raw");
                goto finish;
        }
 
        if (!i)
        if (finish_command(&child)) {
                ret = error("error occurred running diff --raw");
                goto finish;
        }
 
        if (!i)

-               return 0;
+               goto finish;

 
        /*
         * Changes to submodules require special treatment.This loop writes a
 
        /*
         * Changes to submodules require special treatment.This loop writes a


@@ -626,6 +636,9 @@ static int run_dir_diff(const char *extcmd, int symlinks, const char *prefix,
                exit_cleanup(tmpdir, rc);
 
 finish:
                exit_cleanup(tmpdir, rc);
 
 finish:

+       if (fp)
+               fclose(fp);
+

        free(lbase_dir);
        free(rbase_dir);
        strbuf_release(&ldir);
        free(lbase_dir);
        free(rbase_dir);
        strbuf_release(&ldir);