Merge branch 'jk/config-lockfile-leak-fix' into maint
authorJunio C Hamano <gitster@pobox.com>
Wed, 18 Oct 2017 05:19:07 +0000 (14:19 +0900)
committerJunio C Hamano <gitster@pobox.com>
Wed, 18 Oct 2017 05:19:08 +0000 (14:19 +0900)
A leakfix.

* jk/config-lockfile-leak-fix:
config: use a static lock_file struct

1  2 
config.c
diff --combined config.c
index acebc85d81372f15d3d490d84dc751f59bde26c4,ab7e29540d9fac0e99a50f6dee16ddaa6147c810..c138c0ba055bcec33f3a78a5a2227d2ce6cfa4e3
+++ b/config.c
@@@ -6,8 -6,6 +6,8 @@@
   *
   */
  #include "cache.h"
 +#include "config.h"
 +#include "repository.h"
  #include "lockfile.h"
  #include "exec_cmd.h"
  #include "strbuf.h"
@@@ -15,8 -13,6 +15,8 @@@
  #include "hashmap.h"
  #include "string-list.h"
  #include "utf8.h"
 +#include "dir.h"
 +#include "color.h"
  
  struct config_source {
        struct config_source *prev;
@@@ -74,6 -70,13 +74,6 @@@ static int core_compression_seen
  static int pack_compression_seen;
  static int zlib_compression_seen;
  
 -/*
 - * Default config_set that contains key-value pairs from the usual set of config
 - * config files (i.e repo specific .git/config, user wide ~/.gitconfig, XDG
 - * config file and the global /etc/gitconfig)
 - */
 -static struct config_set the_config_set;
 -
  static int config_file_fgetc(struct config_source *conf)
  {
        return getc_unlocked(conf->u.file);
@@@ -131,7 -134,7 +131,7 @@@ static int handle_path_include(const ch
        if (!path)
                return config_error_nonbool("include.path");
  
 -      expanded = expand_user_path(path);
 +      expanded = expand_user_path(path, 0);
        if (!expanded)
                return error("could not expand include path '%s'", path);
        path = expanded;
        return ret;
  }
  
 +static int prepare_include_condition_pattern(struct strbuf *pat)
 +{
 +      struct strbuf path = STRBUF_INIT;
 +      char *expanded;
 +      int prefix = 0;
 +
 +      expanded = expand_user_path(pat->buf, 1);
 +      if (expanded) {
 +              strbuf_reset(pat);
 +              strbuf_addstr(pat, expanded);
 +              free(expanded);
 +      }
 +
 +      if (pat->buf[0] == '.' && is_dir_sep(pat->buf[1])) {
 +              const char *slash;
 +
 +              if (!cf || !cf->path)
 +                      return error(_("relative config include "
 +                                     "conditionals must come from files"));
 +
 +              strbuf_realpath(&path, cf->path, 1);
 +              slash = find_last_dir_sep(path.buf);
 +              if (!slash)
 +                      die("BUG: how is this possible?");
 +              strbuf_splice(pat, 0, 1, path.buf, slash - path.buf);
 +              prefix = slash - path.buf + 1 /* slash */;
 +      } else if (!is_absolute_path(pat->buf))
 +              strbuf_insert(pat, 0, "**/", 3);
 +
 +      if (pat->len && is_dir_sep(pat->buf[pat->len - 1]))
 +              strbuf_addstr(pat, "**");
 +
 +      strbuf_release(&path);
 +      return prefix;
 +}
 +
 +static int include_by_gitdir(const struct config_options *opts,
 +                           const char *cond, size_t cond_len, int icase)
 +{
 +      struct strbuf text = STRBUF_INIT;
 +      struct strbuf pattern = STRBUF_INIT;
 +      int ret = 0, prefix;
 +      const char *git_dir;
 +      int already_tried_absolute = 0;
 +
 +      if (opts->git_dir)
 +              git_dir = opts->git_dir;
 +      else
 +              goto done;
 +
 +      strbuf_realpath(&text, git_dir, 1);
 +      strbuf_add(&pattern, cond, cond_len);
 +      prefix = prepare_include_condition_pattern(&pattern);
 +
 +again:
 +      if (prefix < 0)
 +              goto done;
 +
 +      if (prefix > 0) {
 +              /*
 +               * perform literal matching on the prefix part so that
 +               * any wildcard character in it can't create side effects.
 +               */
 +              if (text.len < prefix)
 +                      goto done;
 +              if (!icase && strncmp(pattern.buf, text.buf, prefix))
 +                      goto done;
 +              if (icase && strncasecmp(pattern.buf, text.buf, prefix))
 +                      goto done;
 +      }
 +
 +      ret = !wildmatch(pattern.buf + prefix, text.buf + prefix,
 +                       icase ? WM_CASEFOLD : 0);
 +
 +      if (!ret && !already_tried_absolute) {
 +              /*
 +               * We've tried e.g. matching gitdir:~/work, but if
 +               * ~/work is a symlink to /mnt/storage/work
 +               * strbuf_realpath() will expand it, so the rule won't
 +               * match. Let's match against a
 +               * strbuf_add_absolute_path() version of the path,
 +               * which'll do the right thing
 +               */
 +              strbuf_reset(&text);
 +              strbuf_add_absolute_path(&text, git_dir);
 +              already_tried_absolute = 1;
 +              goto again;
 +      }
 +done:
 +      strbuf_release(&pattern);
 +      strbuf_release(&text);
 +      return ret;
 +}
 +
 +static int include_condition_is_true(const struct config_options *opts,
 +                                   const char *cond, size_t cond_len)
 +{
 +
 +      if (skip_prefix_mem(cond, cond_len, "gitdir:", &cond, &cond_len))
 +              return include_by_gitdir(opts, cond, cond_len, 0);
 +      else if (skip_prefix_mem(cond, cond_len, "gitdir/i:", &cond, &cond_len))
 +              return include_by_gitdir(opts, cond, cond_len, 1);
 +
 +      /* unknown conditionals are always false */
 +      return 0;
 +}
 +
  int git_config_include(const char *var, const char *value, void *data)
  {
        struct config_include_data *inc = data;
 +      const char *cond, *key;
 +      int cond_len;
        int ret;
  
        /*
  
        if (!strcmp(var, "include.path"))
                ret = handle_path_include(value, inc);
 +
 +      if (!parse_config_key(var, "includeif", &cond, &cond_len, &key) &&
 +          (cond && include_condition_is_true(inc->opts, cond, cond_len)) &&
 +          !strcmp(key, "path"))
 +              ret = handle_path_include(value, inc);
 +
        return ret;
  }
  
@@@ -389,7 -277,8 +389,7 @@@ static int git_config_parse_key_1(cons
  
  out_free_ret_1:
        if (store_key) {
 -              free(*store_key);
 -              *store_key = NULL;
 +              FREE_AND_NULL(*store_key);
        }
        return -CONFIG_INVALID_KEY;
  }
@@@ -597,8 -486,7 +597,8 @@@ static int get_value(config_fn_t fn, vo
         */
        cf->linenr--;
        ret = fn(name->buf, value, data);
 -      cf->linenr++;
 +      if (ret >= 0)
 +              cf->linenr++;
        return ret;
  }
  
@@@ -854,15 -742,6 +854,15 @@@ int git_parse_ulong(const char *value, 
        return 1;
  }
  
 +static int git_parse_ssize_t(const char *value, ssize_t *ret)
 +{
 +      intmax_t tmp;
 +      if (!git_parse_signed(value, &tmp, maximum_signed_value_of_type(ssize_t)))
 +              return 0;
 +      *ret = tmp;
 +      return 1;
 +}
 +
  NORETURN
  static void die_bad_number(const char *name, const char *value)
  {
@@@ -921,14 -800,6 +921,14 @@@ unsigned long git_config_ulong(const ch
        return ret;
  }
  
 +ssize_t git_config_ssize_t(const char *name, const char *value)
 +{
 +      ssize_t ret;
 +      if (!git_parse_ssize_t(value, &ret))
 +              die_bad_number(name, value);
 +      return ret;
 +}
 +
  int git_parse_maybe_bool(const char *value)
  {
        if (!value)
@@@ -985,7 -856,7 +985,7 @@@ int git_config_pathname(const char **de
  {
        if (!value)
                return config_error_nonbool(var);
 -      *dest = expand_user_path(value);
 +      *dest = expand_user_path(value, 0);
        if (!*dest)
                die(_("failed to expand user dir in: '%s'"), value);
        return 0;
@@@ -1351,9 -1222,6 +1351,9 @@@ int git_default_config(const char *var
        if (starts_with(var, "advice."))
                return git_default_advice_config(var, value);
  
 +      if (git_color_config(var, value, dummy) < 0)
 +              return -1;
 +
        if (!strcmp(var, "pager.color") || !strcmp(var, "color.pager")) {
                pager_use_color = git_config_bool(var,value);
                return 0;
@@@ -1435,7 -1303,7 +1435,7 @@@ int git_config_from_file(config_fn_t fn
        int ret = -1;
        FILE *f;
  
 -      f = fopen(filename, "r");
 +      f = fopen_or_warn(filename, "r");
        if (f) {
                flockfile(f);
                ret = do_config_from_file(fn, CONFIG_ORIGIN_FILE, filename, filename, f, data);
@@@ -1534,18 -1402,12 +1534,18 @@@ int git_config_system(void
        return !git_env_bool("GIT_CONFIG_NOSYSTEM", 0);
  }
  
 -static int do_git_config_sequence(config_fn_t fn, void *data)
 +static int do_git_config_sequence(const struct config_options *opts,
 +                                config_fn_t fn, void *data)
  {
        int ret = 0;
        char *xdg_config = xdg_config_home("config");
 -      char *user_config = expand_user_path("~/.gitconfig");
 -      char *repo_config = have_git_dir() ? git_pathdup("config") : NULL;
 +      char *user_config = expand_user_path("~/.gitconfig", 0);
 +      char *repo_config;
 +
 +      if (opts->commondir)
 +              repo_config = mkpathdup("%s/config", opts->commondir);
 +      else
 +              repo_config = NULL;
  
        current_parsing_scope = CONFIG_SCOPE_SYSTEM;
        if (git_config_system() && !access_or_die(git_etc_gitconfig(), R_OK, 0))
        return ret;
  }
  
 -int git_config_with_options(config_fn_t fn, void *data,
 -                          struct git_config_source *config_source,
 -                          int respect_includes)
 +int config_with_options(config_fn_t fn, void *data,
 +                      struct git_config_source *config_source,
 +                      const struct config_options *opts)
  {
        struct config_include_data inc = CONFIG_INCLUDE_INIT;
  
 -      if (respect_includes) {
 +      if (opts->respect_includes) {
                inc.fn = fn;
                inc.data = data;
 +              inc.opts = opts;
                fn = git_config_include;
                data = &inc;
        }
        else if (config_source && config_source->blob)
                return git_config_from_blob_ref(fn, config_source->blob, data);
  
 -      return do_git_config_sequence(fn, data);
 -}
 -
 -static void git_config_raw(config_fn_t fn, void *data)
 -{
 -      if (git_config_with_options(fn, data, NULL, 1) < 0)
 -              /*
 -               * git_config_with_options() normally returns only
 -               * zero, as most errors are fatal, and
 -               * non-fatal potential errors are guarded by "if"
 -               * statements that are entered only when no error is
 -               * possible.
 -               *
 -               * If we ever encounter a non-fatal error, it means
 -               * something went really wrong and we should stop
 -               * immediately.
 -               */
 -              die(_("unknown error occurred while reading the configuration files"));
 +      return do_git_config_sequence(opts, fn, data);
  }
  
  static void configset_iter(struct config_set *cs, config_fn_t fn, void *data)
        }
  }
  
 -static void git_config_check_init(void);
 -
 -void git_config(config_fn_t fn, void *data)
 +void read_early_config(config_fn_t cb, void *data)
  {
 -      git_config_check_init();
 -      configset_iter(&the_config_set, fn, data);
 +      struct config_options opts = {0};
 +      struct strbuf commondir = STRBUF_INIT;
 +      struct strbuf gitdir = STRBUF_INIT;
 +
 +      opts.respect_includes = 1;
 +
 +      if (have_git_dir()) {
 +              opts.commondir = get_git_common_dir();
 +              opts.git_dir = get_git_dir();
 +      /*
 +       * When setup_git_directory() was not yet asked to discover the
 +       * GIT_DIR, we ask discover_git_directory() to figure out whether there
 +       * is any repository config we should use (but unlike
 +       * setup_git_directory_gently(), no global state is changed, most
 +       * notably, the current working directory is still the same after the
 +       * call).
 +       */
 +      } else if (!discover_git_directory(&commondir, &gitdir)) {
 +              opts.commondir = commondir.buf;
 +              opts.git_dir = gitdir.buf;
 +      }
 +
 +      config_with_options(cb, data, NULL, &opts);
 +
 +      strbuf_release(&commondir);
 +      strbuf_release(&gitdir);
  }
  
  static struct config_set_element *configset_find_element(struct config_set *cs, const char *key)
@@@ -1718,18 -1574,15 +1718,18 @@@ static int configset_add_value(struct c
        return 0;
  }
  
 -static int config_set_element_cmp(const struct config_set_element *e1,
 -                               const struct config_set_element *e2, const void *unused)
 +static int config_set_element_cmp(const void *unused_cmp_data,
 +                                const struct config_set_element *e1,
 +                                const struct config_set_element *e2,
 +                                const void *unused_keydata)
  {
        return strcmp(e1->key, e2->key);
  }
  
  void git_configset_init(struct config_set *cs)
  {
 -      hashmap_init(&cs->config_hash, (hashmap_cmp_fn)config_set_element_cmp, 0);
 +      hashmap_init(&cs->config_hash, (hashmap_cmp_fn)config_set_element_cmp,
 +                   NULL, 0);
        cs->hash_initialized = 1;
        cs->list.nr = 0;
        cs->list.alloc = 0;
@@@ -1867,206 -1720,86 +1867,206 @@@ int git_configset_get_pathname(struct c
                return 1;
  }
  
 -static void git_config_check_init(void)
 +/* Functions use to read configuration from a repository */
 +static void repo_read_config(struct repository *repo)
  {
 -      if (the_config_set.hash_initialized)
 +      struct config_options opts;
 +
 +      opts.respect_includes = 1;
 +      opts.commondir = repo->commondir;
 +      opts.git_dir = repo->gitdir;
 +
 +      if (!repo->config)
 +              repo->config = xcalloc(1, sizeof(struct config_set));
 +      else
 +              git_configset_clear(repo->config);
 +
 +      git_configset_init(repo->config);
 +
 +      if (config_with_options(config_set_callback, repo->config, NULL, &opts) < 0)
 +              /*
 +               * config_with_options() normally returns only
 +               * zero, as most errors are fatal, and
 +               * non-fatal potential errors are guarded by "if"
 +               * statements that are entered only when no error is
 +               * possible.
 +               *
 +               * If we ever encounter a non-fatal error, it means
 +               * something went really wrong and we should stop
 +               * immediately.
 +               */
 +              die(_("unknown error occurred while reading the configuration files"));
 +}
 +
 +static void git_config_check_init(struct repository *repo)
 +{
 +      if (repo->config && repo->config->hash_initialized)
                return;
 -      git_configset_init(&the_config_set);
 -      git_config_raw(config_set_callback, &the_config_set);
 +      repo_read_config(repo);
  }
  
 -void git_config_clear(void)
 +static void repo_config_clear(struct repository *repo)
  {
 -      if (!the_config_set.hash_initialized)
 +      if (!repo->config || !repo->config->hash_initialized)
                return;
 -      git_configset_clear(&the_config_set);
 +      git_configset_clear(repo->config);
  }
  
 -int git_config_get_value(const char *key, const char **value)
 +void repo_config(struct repository *repo, config_fn_t fn, void *data)
  {
 -      git_config_check_init();
 -      return git_configset_get_value(&the_config_set, key, value);
 +      git_config_check_init(repo);
 +      configset_iter(repo->config, fn, data);
  }
  
 -const struct string_list *git_config_get_value_multi(const char *key)
 +int repo_config_get_value(struct repository *repo,
 +                        const char *key, const char **value)
  {
 -      git_config_check_init();
 -      return git_configset_get_value_multi(&the_config_set, key);
 +      git_config_check_init(repo);
 +      return git_configset_get_value(repo->config, key, value);
  }
  
 -int git_config_get_string_const(const char *key, const char **dest)
 +const struct string_list *repo_config_get_value_multi(struct repository *repo,
 +                                                    const char *key)
 +{
 +      git_config_check_init(repo);
 +      return git_configset_get_value_multi(repo->config, key);
 +}
 +
 +int repo_config_get_string_const(struct repository *repo,
 +                               const char *key, const char **dest)
 +{
 +      int ret;
 +      git_config_check_init(repo);
 +      ret = git_configset_get_string_const(repo->config, key, dest);
 +      if (ret < 0)
 +              git_die_config(key, NULL);
 +      return ret;
 +}
 +
 +int repo_config_get_string(struct repository *repo,
 +                         const char *key, char **dest)
 +{
 +      git_config_check_init(repo);
 +      return repo_config_get_string_const(repo, key, (const char **)dest);
 +}
 +
 +int repo_config_get_int(struct repository *repo,
 +                      const char *key, int *dest)
 +{
 +      git_config_check_init(repo);
 +      return git_configset_get_int(repo->config, key, dest);
 +}
 +
 +int repo_config_get_ulong(struct repository *repo,
 +                        const char *key, unsigned long *dest)
 +{
 +      git_config_check_init(repo);
 +      return git_configset_get_ulong(repo->config, key, dest);
 +}
 +
 +int repo_config_get_bool(struct repository *repo,
 +                       const char *key, int *dest)
 +{
 +      git_config_check_init(repo);
 +      return git_configset_get_bool(repo->config, key, dest);
 +}
 +
 +int repo_config_get_bool_or_int(struct repository *repo,
 +                              const char *key, int *is_bool, int *dest)
 +{
 +      git_config_check_init(repo);
 +      return git_configset_get_bool_or_int(repo->config, key, is_bool, dest);
 +}
 +
 +int repo_config_get_maybe_bool(struct repository *repo,
 +                             const char *key, int *dest)
 +{
 +      git_config_check_init(repo);
 +      return git_configset_get_maybe_bool(repo->config, key, dest);
 +}
 +
 +int repo_config_get_pathname(struct repository *repo,
 +                           const char *key, const char **dest)
  {
        int ret;
 -      git_config_check_init();
 -      ret = git_configset_get_string_const(&the_config_set, key, dest);
 +      git_config_check_init(repo);
 +      ret = git_configset_get_pathname(repo->config, key, dest);
        if (ret < 0)
                git_die_config(key, NULL);
        return ret;
  }
  
 +/* Functions used historically to read configuration from 'the_repository' */
 +void git_config(config_fn_t fn, void *data)
 +{
 +      repo_config(the_repository, fn, data);
 +}
 +
 +void git_config_clear(void)
 +{
 +      repo_config_clear(the_repository);
 +}
 +
 +int git_config_get_value(const char *key, const char **value)
 +{
 +      return repo_config_get_value(the_repository, key, value);
 +}
 +
 +const struct string_list *git_config_get_value_multi(const char *key)
 +{
 +      return repo_config_get_value_multi(the_repository, key);
 +}
 +
 +int git_config_get_string_const(const char *key, const char **dest)
 +{
 +      return repo_config_get_string_const(the_repository, key, dest);
 +}
 +
  int git_config_get_string(const char *key, char **dest)
  {
 -      git_config_check_init();
 -      return git_config_get_string_const(key, (const char **)dest);
 +      return repo_config_get_string(the_repository, key, dest);
  }
  
  int git_config_get_int(const char *key, int *dest)
  {
 -      git_config_check_init();
 -      return git_configset_get_int(&the_config_set, key, dest);
 +      return repo_config_get_int(the_repository, key, dest);
  }
  
  int git_config_get_ulong(const char *key, unsigned long *dest)
  {
 -      git_config_check_init();
 -      return git_configset_get_ulong(&the_config_set, key, dest);
 +      return repo_config_get_ulong(the_repository, key, dest);
  }
  
  int git_config_get_bool(const char *key, int *dest)
  {
 -      git_config_check_init();
 -      return git_configset_get_bool(&the_config_set, key, dest);
 +      return repo_config_get_bool(the_repository, key, dest);
  }
  
  int git_config_get_bool_or_int(const char *key, int *is_bool, int *dest)
  {
 -      git_config_check_init();
 -      return git_configset_get_bool_or_int(&the_config_set, key, is_bool, dest);
 +      return repo_config_get_bool_or_int(the_repository, key, is_bool, dest);
  }
  
  int git_config_get_maybe_bool(const char *key, int *dest)
  {
 -      git_config_check_init();
 -      return git_configset_get_maybe_bool(&the_config_set, key, dest);
 +      return repo_config_get_maybe_bool(the_repository, key, dest);
  }
  
  int git_config_get_pathname(const char *key, const char **dest)
  {
 -      int ret;
 -      git_config_check_init();
 -      ret = git_configset_get_pathname(&the_config_set, key, dest);
 -      if (ret < 0)
 -              git_die_config(key, NULL);
 +      return repo_config_get_pathname(the_repository, key, dest);
 +}
 +
 +int git_config_get_expiry(const char *key, const char **output)
 +{
 +      int ret = git_config_get_string_const(key, output);
 +      if (ret)
 +              return ret;
 +      if (strcmp(*output, "now")) {
 +              timestamp_t now = approxidate("now");
 +              if (approxidate(*output) >= now)
 +                      git_die_config(key, _("Invalid %s: '%s'"), key, *output);
 +      }
        return ret;
  }
  
@@@ -2086,39 -1819,14 +2086,39 @@@ int git_config_get_untracked_cache(void
                if (!strcasecmp(v, "keep"))
                        return -1;
  
 -              error("unknown core.untrackedCache value '%s'; "
 -                    "using 'keep' default value", v);
 +              error(_("unknown core.untrackedCache value '%s'; "
 +                      "using 'keep' default value"), v);
                return -1;
        }
  
        return -1; /* default value */
  }
  
 +int git_config_get_split_index(void)
 +{
 +      int val;
 +
 +      if (!git_config_get_maybe_bool("core.splitindex", &val))
 +              return val;
 +
 +      return -1; /* default value */
 +}
 +
 +int git_config_get_max_percent_split_change(void)
 +{
 +      int val = -1;
 +
 +      if (!git_config_get_int("splitindex.maxpercentchange", &val)) {
 +              if (0 <= val && val <= 100)
 +                      return val;
 +
 +              return error(_("splitIndex.maxPercentChange value '%d' "
 +                             "should be between 0 and 100"), val);
 +      }
 +
 +      return -1; /* default value */
 +}
 +
  NORETURN
  void git_die_config_linenr(const char *key, const char *filename, int linenr)
  {
@@@ -2404,7 -2112,7 +2404,7 @@@ int git_config_set_multivar_in_file_gen
  {
        int fd = -1, in_fd = -1;
        int ret;
-       struct lock_file *lock = NULL;
+       static struct lock_file lock;
        char *filename_buf = NULL;
        char *contents = NULL;
        size_t contents_sz;
         * The lock serves a purpose in addition to locking: the new
         * contents of .git/config will be written into it.
         */
-       lock = xcalloc(1, sizeof(struct lock_file));
-       fd = hold_lock_file_for_update(lock, config_filename, 0);
+       fd = hold_lock_file_for_update(&lock, config_filename, 0);
        if (fd < 0) {
                error_errno("could not lock config file %s", config_filename);
                free(store.key);
                close(in_fd);
                in_fd = -1;
  
-               if (chmod(get_lock_file_path(lock), st.st_mode & 07777) < 0) {
-                       error_errno("chmod on %s failed", get_lock_file_path(lock));
+               if (chmod(get_lock_file_path(&lock), st.st_mode & 07777) < 0) {
+                       error_errno("chmod on %s failed", get_lock_file_path(&lock));
                        ret = CONFIG_NO_WRITE;
                        goto out_free;
                }
                contents = NULL;
        }
  
-       if (commit_lock_file(lock) < 0) {
+       if (commit_lock_file(&lock) < 0) {
                error_errno("could not write config file %s", config_filename);
                ret = CONFIG_NO_WRITE;
-               lock = NULL;
                goto out_free;
        }
  
-       /*
-        * lock is committed, so don't try to roll it back below.
-        * NOTE: Since lockfile.c keeps a linked list of all created
-        * lock_file structures, it isn't safe to free(lock).  It's
-        * better to just leave it hanging around.
-        */
-       lock = NULL;
        ret = 0;
  
        /* Invalidate the config cache */
        git_config_clear();
  
  out_free:
-       if (lock)
-               rollback_lock_file(lock);
+       rollback_lock_file(&lock);
        free(filename_buf);
        if (contents)
                munmap(contents, contents_sz);
        return ret;
  
  write_err_out:
-       ret = write_error(get_lock_file_path(lock));
+       ret = write_error(get_lock_file_path(&lock));
        goto out_free;
  
  }
@@@ -2719,7 -2417,7 +2709,7 @@@ int git_config_rename_section_in_file(c
        struct lock_file *lock;
        int out_fd;
        char buf[1024];
 -      FILE *config_file;
 +      FILE *config_file = NULL;
        struct stat st;
  
        if (new_name && !section_name_is_ok(new_name)) {
        }
  
        if (!(config_file = fopen(config_filename, "rb"))) {
 +              ret = warn_on_fopen_errors(config_filename);
 +              if (ret)
 +                      goto out;
                /* no config file means nothing to rename, no error */
                goto commit_and_out;
        }
                }
        }
        fclose(config_file);
 +      config_file = NULL;
  commit_and_out:
        if (commit_lock_file(lock) < 0)
                ret = error_errno("could not write config file %s",
                                  config_filename);
  out:
 +      if (config_file)
 +              fclose(config_file);
        rollback_lock_file(lock);
  out_no_rollback:
        free(filename_buf);