Merge branch 'sb/plug-misc-leaks'
authorJunio C Hamano <gitster@pobox.com>
Mon, 25 Jun 2018 20:22:41 +0000 (13:22 -0700)
committerJunio C Hamano <gitster@pobox.com>
Mon, 25 Jun 2018 20:22:41 +0000 (13:22 -0700)
Misc leak plugging.

* sb/plug-misc-leaks:
  sequencer.c: plug mem leak in git_sequencer_config
  sequencer.c: plug leaks in do_pick_commit
  submodule--helper: plug mem leak in print_default_remote
  refs/packed-backend.c: close fd of empty file

1  2 
builtin/submodule--helper.c
sequencer.c

@@@ -55,7 -55,7 +55,7 @@@ static char *get_default_remote(void
  
  static int print_default_remote(int argc, const char **argv, const char *prefix)
  {
-       const char *remote;
+       char *remote;
  
        if (argc != 1)
                die(_("submodule--helper print-default-remote takes no arguments"));
@@@ -64,6 -64,7 +64,7 @@@
        if (remote)
                printf("%s\n", remote);
  
+       free(remote);
        return 0;
  }
  
@@@ -440,149 -441,6 +441,149 @@@ static void for_each_listed_submodule(c
                fn(list->entries[i], cb_data);
  }
  
 +struct cb_foreach {
 +      int argc;
 +      const char **argv;
 +      const char *prefix;
 +      int quiet;
 +      int recursive;
 +};
 +#define CB_FOREACH_INIT { 0 }
 +
 +static void runcommand_in_submodule_cb(const struct cache_entry *list_item,
 +                                     void *cb_data)
 +{
 +      struct cb_foreach *info = cb_data;
 +      const char *path = list_item->name;
 +      const struct object_id *ce_oid = &list_item->oid;
 +
 +      const struct submodule *sub;
 +      struct child_process cp = CHILD_PROCESS_INIT;
 +      char *displaypath;
 +
 +      displaypath = get_submodule_displaypath(path, info->prefix);
 +
 +      sub = submodule_from_path(the_repository, &null_oid, path);
 +
 +      if (!sub)
 +              die(_("No url found for submodule path '%s' in .gitmodules"),
 +                      displaypath);
 +
 +      if (!is_submodule_populated_gently(path, NULL))
 +              goto cleanup;
 +
 +      prepare_submodule_repo_env(&cp.env_array);
 +
 +      /*
 +       * For the purpose of executing <command> in the submodule,
 +       * separate shell is used for the purpose of running the
 +       * child process.
 +       */
 +      cp.use_shell = 1;
 +      cp.dir = path;
 +
 +      /*
 +       * NEEDSWORK: the command currently has access to the variables $name,
 +       * $sm_path, $displaypath, $sha1 and $toplevel only when the command
 +       * contains a single argument. This is done for maintaining a faithful
 +       * translation from shell script.
 +       */
 +      if (info->argc == 1) {
 +              char *toplevel = xgetcwd();
 +              struct strbuf sb = STRBUF_INIT;
 +
 +              argv_array_pushf(&cp.env_array, "name=%s", sub->name);
 +              argv_array_pushf(&cp.env_array, "sm_path=%s", path);
 +              argv_array_pushf(&cp.env_array, "displaypath=%s", displaypath);
 +              argv_array_pushf(&cp.env_array, "sha1=%s",
 +                              oid_to_hex(ce_oid));
 +              argv_array_pushf(&cp.env_array, "toplevel=%s", toplevel);
 +
 +              /*
 +               * Since the path variable was accessible from the script
 +               * before porting, it is also made available after porting.
 +               * The environment variable "PATH" has a very special purpose
 +               * on windows. And since environment variables are
 +               * case-insensitive in windows, it interferes with the
 +               * existing PATH variable. Hence, to avoid that, we expose
 +               * path via the args argv_array and not via env_array.
 +               */
 +              sq_quote_buf(&sb, path);
 +              argv_array_pushf(&cp.args, "path=%s; %s",
 +                               sb.buf, info->argv[0]);
 +              strbuf_release(&sb);
 +              free(toplevel);
 +      } else {
 +              argv_array_pushv(&cp.args, info->argv);
 +      }
 +
 +      if (!info->quiet)
 +              printf(_("Entering '%s'\n"), displaypath);
 +
 +      if (info->argv[0] && run_command(&cp))
 +              die(_("run_command returned non-zero status for %s\n."),
 +                      displaypath);
 +
 +      if (info->recursive) {
 +              struct child_process cpr = CHILD_PROCESS_INIT;
 +
 +              cpr.git_cmd = 1;
 +              cpr.dir = path;
 +              prepare_submodule_repo_env(&cpr.env_array);
 +
 +              argv_array_pushl(&cpr.args, "--super-prefix", NULL);
 +              argv_array_pushf(&cpr.args, "%s/", displaypath);
 +              argv_array_pushl(&cpr.args, "submodule--helper", "foreach", "--recursive",
 +                              NULL);
 +
 +              if (info->quiet)
 +                      argv_array_push(&cpr.args, "--quiet");
 +
 +              argv_array_pushv(&cpr.args, info->argv);
 +
 +              if (run_command(&cpr))
 +                      die(_("run_command returned non-zero status while"
 +                              "recursing in the nested submodules of %s\n."),
 +                              displaypath);
 +      }
 +
 +cleanup:
 +      free(displaypath);
 +}
 +
 +static int module_foreach(int argc, const char **argv, const char *prefix)
 +{
 +      struct cb_foreach info = CB_FOREACH_INIT;
 +      struct pathspec pathspec;
 +      struct module_list list = MODULE_LIST_INIT;
 +
 +      struct option module_foreach_options[] = {
 +              OPT__QUIET(&info.quiet, N_("Suppress output of entering each submodule command")),
 +              OPT_BOOL(0, "recursive", &info.recursive,
 +                       N_("Recurse into nested submodules")),
 +              OPT_END()
 +      };
 +
 +      const char *const git_submodule_helper_usage[] = {
 +              N_("git submodule--helper foreach [--quiet] [--recursive] <command>"),
 +              NULL
 +      };
 +
 +      argc = parse_options(argc, argv, prefix, module_foreach_options,
 +                           git_submodule_helper_usage, PARSE_OPT_KEEP_UNKNOWN);
 +
 +      if (module_list_compute(0, NULL, prefix, &pathspec, &list) < 0)
 +              return 1;
 +
 +      info.argc = argc;
 +      info.argv = argv;
 +      info.prefix = prefix;
 +
 +      for_each_listed_submodule(&list, runcommand_in_submodule_cb, &info);
 +
 +      return 0;
 +}
 +
  struct init_cb {
        const char *prefix;
        unsigned int flags;
@@@ -2019,7 -1877,6 +2020,7 @@@ static struct cmd_struct commands[] = 
        {"relative-path", resolve_relative_path, 0},
        {"resolve-relative-url", resolve_relative_url, 0},
        {"resolve-relative-url-test", resolve_relative_url_test, 0},
 +      {"foreach", module_foreach, SUPPORT_SUPER_PREFIX},
        {"init", module_init, SUPPORT_SUPER_PREFIX},
        {"status", module_status, SUPPORT_SUPER_PREFIX},
        {"print-default-remote", print_default_remote, 0},
diff --combined sequencer.c
@@@ -27,8 -27,6 +27,8 @@@
  #include "worktree.h"
  #include "oidmap.h"
  #include "oidset.h"
 +#include "commit-slab.h"
 +#include "alias.h"
  
  #define GIT_REFLOG_ACTION "GIT_REFLOG_ACTION"
  
@@@ -176,6 -174,7 +176,7 @@@ static int git_sequencer_config(const c
                        warning(_("invalid commit message cleanup mode '%s'"),
                                  s);
  
+               free((char *)s);
                return status;
        }
  
@@@ -785,7 -784,7 +786,7 @@@ static int run_git_commit(const char *d
        struct child_process cmd = CHILD_PROCESS_INIT;
        const char *value;
  
 -      if (flags & CREATE_ROOT_COMMIT) {
 +      if ((flags & CREATE_ROOT_COMMIT) && !(flags & AMEND_MSG)) {
                struct strbuf msg = STRBUF_INIT, script = STRBUF_INIT;
                const char *author = is_rebase_i(opts) ?
                        read_author_ident(&script) : NULL;
@@@ -1774,7 -1773,8 +1775,8 @@@ static int do_pick_commit(enum todo_com
                res = do_recursive_merge(base, next, base_label, next_label,
                                         &head, &msgbuf, opts);
                if (res < 0)
-                       return res;
+                       goto leave;
                res |= write_message(msgbuf.buf, msgbuf.len,
                                     git_path_merge_msg(), 0);
        } else {
@@@ -3793,7 -3793,7 +3795,7 @@@ static const char *label_oid(struct obj
                                p[i] = save;
                        }
                }
 -      } else if (((len = strlen(label)) == GIT_SHA1_RAWSZ &&
 +      } else if (((len = strlen(label)) == the_hash_algo->hexsz &&
                    !get_oid_hex(label, &dummy)) ||
                   (len == 1 && *label == '#') ||
                   hashmap_get_from_hash(&state->labels,
@@@ -4239,7 -4239,6 +4241,7 @@@ static enum check_level get_missing_com
        return CHECK_IGNORE;
  }
  
 +define_commit_slab(commit_seen, unsigned char);
  /*
   * Check if the user dropped some commits by mistake
   * Behaviour determined by rebase.missingCommitsCheck.
@@@ -4253,9 -4252,6 +4255,9 @@@ int check_todo_list(void
        struct todo_list todo_list = TODO_LIST_INIT;
        struct strbuf missing = STRBUF_INIT;
        int advise_to_edit_todo = 0, res = 0, i;
 +      struct commit_seen commit_seen;
 +
 +      init_commit_seen(&commit_seen);
  
        strbuf_addstr(&todo_file, rebase_path_todo());
        if (strbuf_read_file_or_whine(&todo_list.buf, todo_file.buf) < 0) {
        for (i = 0; i < todo_list.nr; i++) {
                struct commit *commit = todo_list.items[i].commit;
                if (commit)
 -                      commit->util = (void *)1;
 +                      *commit_seen_at(&commit_seen, commit) = 1;
        }
  
        todo_list_release(&todo_list);
        for (i = todo_list.nr - 1; i >= 0; i--) {
                struct todo_item *item = todo_list.items + i;
                struct commit *commit = item->commit;
 -              if (commit && !commit->util) {
 +              if (commit && !*commit_seen_at(&commit_seen, commit)) {
                        strbuf_addf(&missing, " - %s %.*s\n",
                                    short_commit_name(commit),
                                    item->arg_len, item->arg);
 -                      commit->util = (void *)1;
 +                      *commit_seen_at(&commit_seen, commit) = 1;
                }
        }
  
                "The possible behaviours are: ignore, warn, error.\n\n"));
  
  leave_check:
 +      clear_commit_seen(&commit_seen);
        strbuf_release(&todo_file);
        todo_list_release(&todo_list);
  
@@@ -4439,8 -4434,6 +4441,8 @@@ static int subject2item_cmp(const void 
        return key ? strcmp(a->subject, key) : strcmp(a->subject, b->subject);
  }
  
 +define_commit_slab(commit_todo_item, struct todo_item *);
 +
  /*
   * Rearrange the todo list that has both "pick commit-id msg" and "pick
   * commit-id fixup!/squash! msg" in it so that the latter is put immediately
@@@ -4457,7 -4450,6 +4459,7 @@@ int rearrange_squash(void
        struct hashmap subject2item;
        int res = 0, rearranged = 0, *next, *tail, i;
        char **subjects;
 +      struct commit_todo_item commit_todo;
  
        if (strbuf_read_file_or_whine(&todo_list.buf, todo_file) < 0)
                return -1;
                return -1;
        }
  
 +      init_commit_todo_item(&commit_todo);
        /*
         * The hashmap maps onelines to the respective todo list index.
         *
  
                if (is_fixup(item->command)) {
                        todo_list_release(&todo_list);
 +                      clear_commit_todo_item(&commit_todo);
                        return error(_("the script was already rearranged."));
                }
  
 -              item->commit->util = item;
 +              *commit_todo_item_at(&commit_todo, item->commit) = item;
  
                parse_commit(item->commit);
                commit_buffer = get_commit_buffer(item->commit, NULL);
                        else if (!strchr(p, ' ') &&
                                 (commit2 =
                                  lookup_commit_reference_by_name(p)) &&
 -                               commit2->util)
 +                               *commit_todo_item_at(&commit_todo, commit2))
                                /* found by commit name */
 -                              i2 = (struct todo_item *)commit2->util
 +                              i2 = *commit_todo_item_at(&commit_todo, commit2)
                                        - todo_list.items;
                        else {
                                /* copy can be a prefix of the commit subject */
        hashmap_free(&subject2item, 1);
        todo_list_release(&todo_list);
  
 +      clear_commit_todo_item(&commit_todo);
        return res;
  }