Merge branch 'ma/lockfile-fixes'
authorJunio C Hamano <gitster@pobox.com>
Mon, 6 Nov 2017 04:11:21 +0000 (13:11 +0900)
committerJunio C Hamano <gitster@pobox.com>
Mon, 6 Nov 2017 04:11:21 +0000 (13:11 +0900)
An earlier update made it possible to use an on-stack in-core
lockfile structure (as opposed to having to deliberately leak an
on-heap one). Many codepaths have been updated to take advantage
of this new facility.

* ma/lockfile-fixes:
read_cache: roll back lock in `update_index_if_able()`
read-cache: leave lock in right state in `write_locked_index()`
read-cache: drop explicit `CLOSE_LOCK`-flag
cache.h: document `write_locked_index()`
apply: remove `newfd` from `struct apply_state`
apply: move lockfile into `apply_state`
cache-tree: simplify locking logic
checkout-index: simplify locking logic
tempfile: fix documentation on `delete_tempfile()`
lockfile: fix documentation on `close_lock_file_gently()`
treewide: prefer lockfiles on the stack
sha1_file: do not leak `lock_file`

1  2 
builtin/checkout.c
builtin/commit.c
builtin/diff.c
cache-tree.c
cache.h
config.c
sequencer.c
sha1_file.c
wt-status.c
diff --combined builtin/checkout.c
index fc4f8fd2ea29c7c23d3b1c7ca0ba78824c255a91,34c3c5b615970205876eb1a508c8aecf78961aa6..fcff0de198edb05ca58ab538e12aa77371425e17
@@@ -247,7 -247,7 +247,7 @@@ static int checkout_paths(const struct 
        struct object_id rev;
        struct commit *head;
        int errs = 0;
-       struct lock_file *lock_file;
+       struct lock_file lock_file = LOCK_INIT;
  
        if (opts->track != BRANCH_TRACK_UNSPECIFIED)
                die(_("'%s' cannot be used with updating paths"), "--track");
                return run_add_interactive(revision, "--patch=checkout",
                                           &opts->pathspec);
  
-       lock_file = xcalloc(1, sizeof(struct lock_file));
-       hold_locked_index(lock_file, LOCK_DIE_ON_ERROR);
+       hold_locked_index(&lock_file, LOCK_DIE_ON_ERROR);
        if (read_cache_preload(&opts->pathspec) < 0)
                return error(_("index file corrupt"));
  
        }
        errs |= finish_delayed_checkout(&state);
  
-       if (write_locked_index(&the_index, lock_file, COMMIT_LOCK))
+       if (write_locked_index(&the_index, &lock_file, COMMIT_LOCK))
                die(_("unable to write new index file"));
  
        read_ref_full("HEAD", 0, rev.hash, NULL);
@@@ -472,9 -470,9 +470,9 @@@ static int merge_working_tree(const str
                              int *writeout_error)
  {
        int ret;
-       struct lock_file *lock_file = xcalloc(1, sizeof(struct lock_file));
+       struct lock_file lock_file = LOCK_INIT;
  
-       hold_locked_index(lock_file, LOCK_DIE_ON_ERROR);
+       hold_locked_index(&lock_file, LOCK_DIE_ON_ERROR);
        if (read_cache_preload(NULL) < 0)
                return error(_("index file corrupt"));
  
        if (!cache_tree_fully_valid(active_cache_tree))
                cache_tree_update(&the_index, WRITE_TREE_SILENT | WRITE_TREE_REPAIR);
  
-       if (write_locked_index(&the_index, lock_file, COMMIT_LOCK))
+       if (write_locked_index(&the_index, &lock_file, COMMIT_LOCK))
                die(_("unable to write new index file"));
  
        if (!opts->force && !opts->quiet)
@@@ -1124,8 -1122,9 +1122,8 @@@ static int checkout_branch(struct check
  
        if (new->path && !opts->force_detach && !opts->new_branch &&
            !opts->ignore_other_worktrees) {
 -              struct object_id oid;
                int flag;
 -              char *head_ref = resolve_refdup("HEAD", 0, oid.hash, &flag);
 +              char *head_ref = resolve_refdup("HEAD", 0, NULL, &flag);
                if (head_ref &&
                    (!(flag & REF_ISSYMREF) || strcmp(head_ref, new->path)))
                        die_if_checked_out(new->path, 1);
@@@ -1297,7 -1296,6 +1295,7 @@@ int cmd_checkout(int argc, const char *
                strbuf_release(&buf);
        }
  
 +      UNLEAK(opts);
        if (opts.patch_mode || opts.pathspec.nr)
                return checkout_paths(&opts, new.name);
        else
diff --combined builtin/commit.c
index d75b3805ea7fe3475564f337bf660d8155909445,32dc2101f5c23763ae81292ffa98146d11e607de..af034553fcf22c463d2be04f4aafff7c9d6176dc
@@@ -355,7 -355,7 +355,7 @@@ static const char *prepare_index(int ar
  
                refresh_cache_or_die(refresh_flags);
  
-               if (write_locked_index(&the_index, &index_lock, CLOSE_LOCK))
+               if (write_locked_index(&the_index, &index_lock, 0))
                        die(_("unable to create temporary index"));
  
                old_index_env = getenv(INDEX_ENVIRONMENT);
                if (update_main_cache_tree(WRITE_TREE_SILENT) == 0) {
                        if (reopen_lock_file(&index_lock) < 0)
                                die(_("unable to write index file"));
-                       if (write_locked_index(&the_index, &index_lock, CLOSE_LOCK))
+                       if (write_locked_index(&the_index, &index_lock, 0))
                                die(_("unable to update temporary index"));
                } else
                        warning(_("Failed to update main cache tree"));
                add_files_to_cache(also ? prefix : NULL, &pathspec, 0);
                refresh_cache_or_die(refresh_flags);
                update_main_cache_tree(WRITE_TREE_SILENT);
-               if (write_locked_index(&the_index, &index_lock, CLOSE_LOCK))
+               if (write_locked_index(&the_index, &index_lock, 0))
                        die(_("unable to write new_index file"));
                commit_style = COMMIT_NORMAL;
                ret = get_lock_file_path(&index_lock);
        add_remove_files(&partial);
        refresh_cache(REFRESH_QUIET);
        update_main_cache_tree(WRITE_TREE_SILENT);
-       if (write_locked_index(&the_index, &index_lock, CLOSE_LOCK))
+       if (write_locked_index(&the_index, &index_lock, 0))
                die(_("unable to write new_index file"));
  
        hold_lock_file_for_update(&false_lock,
        add_remove_files(&partial);
        refresh_cache(REFRESH_QUIET);
  
-       if (write_locked_index(&the_index, &false_lock, CLOSE_LOCK))
+       if (write_locked_index(&the_index, &false_lock, 0))
                die(_("unable to write temporary index file"));
  
        discard_cache();
@@@ -1392,10 -1392,7 +1392,10 @@@ int cmd_status(int argc, const char **a
        read_cache_preload(&s.pathspec);
        refresh_index(&the_index, REFRESH_QUIET|REFRESH_UNMERGED, &s.pathspec, NULL, NULL);
  
 -      fd = hold_locked_index(&index_lock, 0);
 +      if (use_optional_locks())
 +              fd = hold_locked_index(&index_lock, 0);
 +      else
 +              fd = -1;
  
        s.is_initial = get_oid(s.reference, &oid) ? 1 : 0;
        if (!s.is_initial)
diff --combined builtin/diff.c
index f5bbd4d757e12e6e34a86fac006298acb771909f,91995965d79d559c7fbea2aa2aedd76ce98b6e96..aa6f746795fb82feb4de1ff28f8f0350ee18092c
@@@ -203,17 -203,16 +203,16 @@@ static int builtin_diff_combined(struc
  
  static void refresh_index_quietly(void)
  {
-       struct lock_file *lock_file;
+       struct lock_file lock_file = LOCK_INIT;
        int fd;
  
-       lock_file = xcalloc(1, sizeof(struct lock_file));
-       fd = hold_locked_index(lock_file, 0);
+       fd = hold_locked_index(&lock_file, 0);
        if (fd < 0)
                return;
        discard_cache();
        read_cache();
        refresh_cache(REFRESH_QUIET|REFRESH_UNMERGED);
-       update_index_if_able(&the_index, lock_file);
+       update_index_if_able(&the_index, &lock_file);
  }
  
  static int builtin_diff_files(struct rev_info *revs, int argc, const char **argv)
@@@ -464,8 -463,5 +463,8 @@@ int cmd_diff(int argc, const char **arg
        result = diff_result_code(&rev.diffopt, result);
        if (1 < rev.diffopt.skip_stat_unmatch)
                refresh_index_quietly();
 +      UNLEAK(rev);
 +      UNLEAK(ent);
 +      UNLEAK(blob);
        return result;
  }
diff --combined cache-tree.c
index d3f7401278bd5130558dde18da1235c188bb7303,f646f567317a3eefcfcd60484d15f319e22549a4..e03e72c34a5c1fc618994ee63f38875d28d91886
@@@ -49,7 -49,7 +49,7 @@@ static int subtree_pos(struct cache_tre
        lo = 0;
        hi = it->subtree_nr;
        while (lo < hi) {
 -              int mi = (lo + hi) / 2;
 +              int mi = lo + (hi - lo) / 2;
                struct cache_tree_sub *mdl = down[mi];
                int cmp = subtree_name_cmp(path, pathlen,
                                           mdl->name, mdl->namelen);
@@@ -602,11 -602,11 +602,11 @@@ static struct cache_tree *cache_tree_fi
  
  int write_index_as_tree(unsigned char *sha1, struct index_state *index_state, const char *index_path, int flags, const char *prefix)
  {
-       int entries, was_valid, newfd;
+       int entries, was_valid;
        struct lock_file lock_file = LOCK_INIT;
        int ret = 0;
  
-       newfd = hold_lock_file_for_update(&lock_file, index_path, LOCK_DIE_ON_ERROR);
+       hold_lock_file_for_update(&lock_file, index_path, LOCK_DIE_ON_ERROR);
  
        entries = read_index_from(index_state, index_path);
        if (entries < 0) {
                        ret = WRITE_TREE_UNMERGED_INDEX;
                        goto out;
                }
-               if (0 <= newfd) {
-                       if (!write_locked_index(index_state, &lock_file, COMMIT_LOCK))
-                               newfd = -1;
-               }
+               write_locked_index(index_state, &lock_file, COMMIT_LOCK);
                /* Not being able to write is fine -- we are only interested
                 * in updating the cache-tree part, and if the next caller
                 * ends up using the old index with unupdated cache-tree part
                hashcpy(sha1, index_state->cache_tree->oid.hash);
  
  out:
-       if (0 <= newfd)
-               rollback_lock_file(&lock_file);
+       rollback_lock_file(&lock_file);
        return ret;
  }
  
diff --combined cache.h
index 6440e2bf21f5800db98f82fcedff46478188674e,8c24937669ed66a7f10c1c57c9a51902023823ea..d74f00d8db2035c5109e2bf00a1ea25c14c7f24c
+++ b/cache.h
@@@ -444,7 -444,6 +444,7 @@@ static inline enum object_type object_t
  #define GIT_NOGLOB_PATHSPECS_ENVIRONMENT "GIT_NOGLOB_PATHSPECS"
  #define GIT_ICASE_PATHSPECS_ENVIRONMENT "GIT_ICASE_PATHSPECS"
  #define GIT_QUARANTINE_ENVIRONMENT "GIT_QUARANTINE_PATH"
 +#define GIT_OPTIONAL_LOCKS_ENVIRONMENT "GIT_OPTIONAL_LOCKS"
  
  /*
   * This environment variable is expected to contain a boolean indicating
@@@ -602,9 -601,28 +602,28 @@@ extern int do_read_index(struct index_s
  extern int read_index_from(struct index_state *, const char *path);
  extern int is_index_unborn(struct index_state *);
  extern int read_index_unmerged(struct index_state *);
+ /* For use with `write_locked_index()`. */
  #define COMMIT_LOCK           (1 << 0)
- #define CLOSE_LOCK            (1 << 1)
+ /*
+  * Write the index while holding an already-taken lock. Close the lock,
+  * and if `COMMIT_LOCK` is given, commit it.
+  *
+  * Unless a split index is in use, write the index into the lockfile.
+  *
+  * With a split index, write the shared index to a temporary file,
+  * adjust its permissions and rename it into place, then write the
+  * split index to the lockfile. If the temporary file for the shared
+  * index cannot be created, fall back to the behavior described in
+  * the previous paragraph.
+  *
+  * With `COMMIT_LOCK`, the lock is always committed or rolled back.
+  * Without it, the lock is closed, but neither committed nor rolled
+  * back.
+  */
  extern int write_locked_index(struct index_state *, struct lock_file *lock, unsigned flags);
  extern int discard_index(struct index_state *);
  extern void move_index_extensions(struct index_state *dst, struct index_state *src);
  extern int unmerged_index(const struct index_state *);
@@@ -716,6 -734,10 +735,10 @@@ extern void fill_stat_cache_info(struc
  extern int refresh_index(struct index_state *, unsigned int flags, const struct pathspec *pathspec, char *seen, const char *header_msg);
  extern struct cache_entry *refresh_cache_entry(struct cache_entry *, unsigned int);
  
+ /*
+  * Opportunistically update the index but do not complain if we can't.
+  * The lockfile is always committed or rolled back.
+  */
  extern void update_index_if_able(struct index_state *, struct lock_file *);
  
  extern int hold_locked_index(struct lock_file *, int);
@@@ -784,11 -806,6 +807,11 @@@ extern int protect_ntfs
   */
  extern int ref_paranoia;
  
 +/*
 + * Returns the boolean value of $GIT_OPTIONAL_LOCKS (or the default value).
 + */
 +int use_optional_locks(void);
 +
  /*
   * The character that begins a commented line in user-editable file
   * that is subject to stripspace.
diff --combined config.c
index adb7d7a3e5ee164d24f1d2420677a8ad5b61e49f,0e61e863d680969cf816950e62a204fa6d50b673..903abf9533b188fd472c213c29a9f968eb90eb8b
+++ b/config.c
@@@ -16,6 -16,7 +16,6 @@@
  #include "string-list.h"
  #include "utf8.h"
  #include "dir.h"
 -#include "color.h"
  
  struct config_source {
        struct config_source *prev;
@@@ -1350,6 -1351,9 +1350,6 @@@ 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;
@@@ -2288,10 -2292,11 +2288,10 @@@ static int write_error(const char *file
        return 4;
  }
  
 -static ssize_t write_section(int fd, const char *key)
 +static struct strbuf store_create_section(const char *key)
  {
        const char *dot;
        int i;
 -      ssize_t ret;
        struct strbuf sb = STRBUF_INIT;
  
        dot = memchr(key, '.', store.baselen);
                strbuf_addf(&sb, "[%.*s]\n", store.baselen, key);
        }
  
 -      ret = write_in_full(fd, sb.buf, sb.len);
 +      return sb;
 +}
 +
 +static ssize_t write_section(int fd, const char *key)
 +{
 +      struct strbuf sb = store_create_section(key);
 +      ssize_t ret;
 +
 +      ret = write_in_full(fd, sb.buf, sb.len) == sb.len;
        strbuf_release(&sb);
  
        return ret;
@@@ -2746,17 -2743,16 +2746,17 @@@ static int section_name_is_ok(const cha
  }
  
  /* if new_name == NULL, the section is removed instead */
 -int git_config_rename_section_in_file(const char *config_filename,
 -                                    const char *old_name, const char *new_name)
 +static int git_config_copy_or_rename_section_in_file(const char *config_filename,
 +                                    const char *old_name, const char *new_name, int copy)
  {
        int ret = 0, remove = 0;
        char *filename_buf = NULL;
-       struct lock_file *lock;
+       struct lock_file lock = LOCK_INIT;
        int out_fd;
        char buf[1024];
        FILE *config_file = NULL;
        struct stat st;
 +      struct strbuf copystr = STRBUF_INIT;
  
        if (new_name && !section_name_is_ok(new_name)) {
                ret = error("invalid section name: %s", new_name);
        if (!config_filename)
                config_filename = filename_buf = git_pathdup("config");
  
-       lock = xcalloc(1, sizeof(struct lock_file));
-       out_fd = hold_lock_file_for_update(lock, config_filename, 0);
+       out_fd = hold_lock_file_for_update(&lock, config_filename, 0);
        if (out_fd < 0) {
                ret = error("could not lock config file %s", config_filename);
                goto out;
                goto out;
        }
  
-       if (chmod(get_lock_file_path(lock), st.st_mode & 07777) < 0) {
+       if (chmod(get_lock_file_path(&lock), st.st_mode & 07777) < 0) {
                ret = error_errno("chmod on %s failed",
-                                 get_lock_file_path(lock));
+                                 get_lock_file_path(&lock));
                goto out;
        }
  
        while (fgets(buf, sizeof(buf), config_file)) {
                int i;
                int length;
 +              int is_section = 0;
                char *output = buf;
                for (i = 0; buf[i] && isspace(buf[i]); i++)
                        ; /* do nothing */
                if (buf[i] == '[') {
                        /* it's a section */
 -                      int offset = section_name_match(&buf[i], old_name);
 +                      int offset;
 +                      is_section = 1;
 +
 +                      /*
 +                       * When encountering a new section under -c we
 +                       * need to flush out any section we're already
 +                       * coping and begin anew. There might be
 +                       * multiple [branch "$name"] sections.
 +                       */
 +                      if (copystr.len > 0) {
 +                              if (write_in_full(out_fd, copystr.buf, copystr.len) != copystr.len) {
-                                       ret = write_error(get_lock_file_path(lock));
++                                      ret = write_error(get_lock_file_path(&lock));
 +                                      goto out;
 +                              }
 +                              strbuf_reset(&copystr);
 +                      }
 +
 +                      offset = section_name_match(&buf[i], old_name);
                        if (offset > 0) {
                                ret++;
                                if (new_name == NULL) {
                                        continue;
                                }
                                store.baselen = strlen(new_name);
 -                              if (write_section(out_fd, new_name) < 0) {
 -                                      ret = write_error(get_lock_file_path(&lock));
 -                                      goto out;
 -                              }
 -                              /*
 -                               * We wrote out the new section, with
 -                               * a newline, now skip the old
 -                               * section's length
 -                               */
 -                              output += offset + i;
 -                              if (strlen(output) > 0) {
 +                              if (!copy) {
 +                                      if (write_section(out_fd, new_name) < 0) {
-                                               ret = write_error(get_lock_file_path(lock));
++                                              ret = write_error(get_lock_file_path(&lock));
 +                                              goto out;
 +                                      }
                                        /*
 -                                       * More content means there's
 -                                       * a declaration to put on the
 -                                       * next line; indent with a
 -                                       * tab
 +                                       * We wrote out the new section, with
 +                                       * a newline, now skip the old
 +                                       * section's length
                                         */
 -                                      output -= 1;
 -                                      output[0] = '\t';
 +                                      output += offset + i;
 +                                      if (strlen(output) > 0) {
 +                                              /*
 +                                               * More content means there's
 +                                               * a declaration to put on the
 +                                               * next line; indent with a
 +                                               * tab
 +                                               */
 +                                              output -= 1;
 +                                              output[0] = '\t';
 +                                      }
 +                              } else {
 +                                      copystr = store_create_section(new_name);
                                }
                        }
                        remove = 0;
                if (remove)
                        continue;
                length = strlen(output);
 +
 +              if (!is_section && copystr.len > 0) {
 +                      strbuf_add(&copystr, output, length);
 +              }
 +
                if (write_in_full(out_fd, output, length) < 0) {
-                       ret = write_error(get_lock_file_path(lock));
+                       ret = write_error(get_lock_file_path(&lock));
                        goto out;
                }
        }
-                       ret = write_error(get_lock_file_path(lock));
 +
 +      /*
 +       * Copy a trailing section at the end of the config, won't be
 +       * flushed by the usual "flush because we have a new section
 +       * logic in the loop above.
 +       */
 +      if (copystr.len > 0) {
 +              if (write_in_full(out_fd, copystr.buf, copystr.len) != copystr.len) {
++                      ret = write_error(get_lock_file_path(&lock));
 +                      goto out;
 +              }
 +              strbuf_reset(&copystr);
 +      }
 +
        fclose(config_file);
        config_file = NULL;
  commit_and_out:
-       if (commit_lock_file(lock) < 0)
+       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);
+       rollback_lock_file(&lock);
  out_no_rollback:
        free(filename_buf);
        return ret;
  }
  
 +int git_config_rename_section_in_file(const char *config_filename,
 +                                    const char *old_name, const char *new_name)
 +{
 +      return git_config_copy_or_rename_section_in_file(config_filename,
 +                                       old_name, new_name, 0);
 +}
 +
  int git_config_rename_section(const char *old_name, const char *new_name)
  {
        return git_config_rename_section_in_file(NULL, old_name, new_name);
  }
  
 +int git_config_copy_section_in_file(const char *config_filename,
 +                                    const char *old_name, const char *new_name)
 +{
 +      return git_config_copy_or_rename_section_in_file(config_filename,
 +                                       old_name, new_name, 1);
 +}
 +
 +int git_config_copy_section(const char *old_name, const char *new_name)
 +{
 +      return git_config_copy_section_in_file(NULL, old_name, new_name);
 +}
 +
  /*
   * Call this to report error for your variable that should not
   * get a boolean value (i.e. "[my] var" means "true").
diff --combined sequencer.c
index f2a10cc4f24c10adf072eb26ba427f00037b3c79,d56c3808198892f8088b2d70b6439e11d01624d0..46c997e2df50f81662df877384fbfc5287fa6d10
@@@ -20,7 -20,6 +20,7 @@@
  #include "trailer.h"
  #include "log-tree.h"
  #include "wt-status.h"
 +#include "hashmap.h"
  
  #define GIT_REFLOG_ACTION "GIT_REFLOG_ACTION"
  
@@@ -204,7 -203,7 +204,7 @@@ int sequencer_remove_state(struct repla
                free(opts->xopts[i]);
        free(opts->xopts);
  
 -      strbuf_addf(&dir, "%s", get_dir(opts));
 +      strbuf_addstr(&dir, get_dir(opts));
        remove_dir_recursively(&dir, 0);
        strbuf_release(&dir);
  
@@@ -1184,7 -1183,6 +1184,6 @@@ static int read_and_refresh_cache(struc
        refresh_index(&the_index, REFRESH_QUIET|REFRESH_UNMERGED, NULL, NULL, NULL);
        if (the_index.cache_changed && index_fd >= 0) {
                if (write_locked_index(&the_index, &index_lock, COMMIT_LOCK)) {
-                       rollback_lock_file(&index_lock);
                        return error(_("git %s: failed to refresh the index"),
                                _(action_name(opts)));
                }
@@@ -2436,533 -2434,3 +2435,533 @@@ void append_signoff(struct strbuf *msgb
  
        strbuf_release(&sob);
  }
 +
 +int sequencer_make_script(int keep_empty, FILE *out,
 +              int argc, const char **argv)
 +{
 +      char *format = NULL;
 +      struct pretty_print_context pp = {0};
 +      struct strbuf buf = STRBUF_INIT;
 +      struct rev_info revs;
 +      struct commit *commit;
 +
 +      init_revisions(&revs, NULL);
 +      revs.verbose_header = 1;
 +      revs.max_parents = 1;
 +      revs.cherry_pick = 1;
 +      revs.limited = 1;
 +      revs.reverse = 1;
 +      revs.right_only = 1;
 +      revs.sort_order = REV_SORT_IN_GRAPH_ORDER;
 +      revs.topo_order = 1;
 +
 +      revs.pretty_given = 1;
 +      git_config_get_string("rebase.instructionFormat", &format);
 +      if (!format || !*format) {
 +              free(format);
 +              format = xstrdup("%s");
 +      }
 +      get_commit_format(format, &revs);
 +      free(format);
 +      pp.fmt = revs.commit_format;
 +      pp.output_encoding = get_log_output_encoding();
 +
 +      if (setup_revisions(argc, argv, &revs, NULL) > 1)
 +              return error(_("make_script: unhandled options"));
 +
 +      if (prepare_revision_walk(&revs) < 0)
 +              return error(_("make_script: error preparing revisions"));
 +
 +      while ((commit = get_revision(&revs))) {
 +              strbuf_reset(&buf);
 +              if (!keep_empty && is_original_commit_empty(commit))
 +                      strbuf_addf(&buf, "%c ", comment_line_char);
 +              strbuf_addf(&buf, "pick %s ", oid_to_hex(&commit->object.oid));
 +              pretty_print_commit(&pp, commit, &buf);
 +              strbuf_addch(&buf, '\n');
 +              fputs(buf.buf, out);
 +      }
 +      strbuf_release(&buf);
 +      return 0;
 +}
 +
 +
 +int transform_todo_ids(int shorten_ids)
 +{
 +      const char *todo_file = rebase_path_todo();
 +      struct todo_list todo_list = TODO_LIST_INIT;
 +      int fd, res, i;
 +      FILE *out;
 +
 +      strbuf_reset(&todo_list.buf);
 +      fd = open(todo_file, O_RDONLY);
 +      if (fd < 0)
 +              return error_errno(_("could not open '%s'"), todo_file);
 +      if (strbuf_read(&todo_list.buf, fd, 0) < 0) {
 +              close(fd);
 +              return error(_("could not read '%s'."), todo_file);
 +      }
 +      close(fd);
 +
 +      res = parse_insn_buffer(todo_list.buf.buf, &todo_list);
 +      if (res) {
 +              todo_list_release(&todo_list);
 +              return error(_("unusable todo list: '%s'"), todo_file);
 +      }
 +
 +      out = fopen(todo_file, "w");
 +      if (!out) {
 +              todo_list_release(&todo_list);
 +              return error(_("unable to open '%s' for writing"), todo_file);
 +      }
 +      for (i = 0; i < todo_list.nr; i++) {
 +              struct todo_item *item = todo_list.items + i;
 +              int bol = item->offset_in_buf;
 +              const char *p = todo_list.buf.buf + bol;
 +              int eol = i + 1 < todo_list.nr ?
 +                      todo_list.items[i + 1].offset_in_buf :
 +                      todo_list.buf.len;
 +
 +              if (item->command >= TODO_EXEC && item->command != TODO_DROP)
 +                      fwrite(p, eol - bol, 1, out);
 +              else {
 +                      const char *id = shorten_ids ?
 +                              short_commit_name(item->commit) :
 +                              oid_to_hex(&item->commit->object.oid);
 +                      int len;
 +
 +                      p += strspn(p, " \t"); /* left-trim command */
 +                      len = strcspn(p, " \t"); /* length of command */
 +
 +                      fprintf(out, "%.*s %s %.*s\n",
 +                              len, p, id, item->arg_len, item->arg);
 +              }
 +      }
 +      fclose(out);
 +      todo_list_release(&todo_list);
 +      return 0;
 +}
 +
 +enum check_level {
 +      CHECK_IGNORE = 0, CHECK_WARN, CHECK_ERROR
 +};
 +
 +static enum check_level get_missing_commit_check_level(void)
 +{
 +      const char *value;
 +
 +      if (git_config_get_value("rebase.missingcommitscheck", &value) ||
 +                      !strcasecmp("ignore", value))
 +              return CHECK_IGNORE;
 +      if (!strcasecmp("warn", value))
 +              return CHECK_WARN;
 +      if (!strcasecmp("error", value))
 +              return CHECK_ERROR;
 +      warning(_("unrecognized setting %s for option "
 +                "rebase.missingCommitsCheck. Ignoring."), value);
 +      return CHECK_IGNORE;
 +}
 +
 +/*
 + * Check if the user dropped some commits by mistake
 + * Behaviour determined by rebase.missingCommitsCheck.
 + * Check if there is an unrecognized command or a
 + * bad SHA-1 in a command.
 + */
 +int check_todo_list(void)
 +{
 +      enum check_level check_level = get_missing_commit_check_level();
 +      struct strbuf todo_file = STRBUF_INIT;
 +      struct todo_list todo_list = TODO_LIST_INIT;
 +      struct strbuf missing = STRBUF_INIT;
 +      int advise_to_edit_todo = 0, res = 0, fd, i;
 +
 +      strbuf_addstr(&todo_file, rebase_path_todo());
 +      fd = open(todo_file.buf, O_RDONLY);
 +      if (fd < 0) {
 +              res = error_errno(_("could not open '%s'"), todo_file.buf);
 +              goto leave_check;
 +      }
 +      if (strbuf_read(&todo_list.buf, fd, 0) < 0) {
 +              close(fd);
 +              res = error(_("could not read '%s'."), todo_file.buf);
 +              goto leave_check;
 +      }
 +      close(fd);
 +      advise_to_edit_todo = res =
 +              parse_insn_buffer(todo_list.buf.buf, &todo_list);
 +
 +      if (res || check_level == CHECK_IGNORE)
 +              goto leave_check;
 +
 +      /* Mark the commits in git-rebase-todo as seen */
 +      for (i = 0; i < todo_list.nr; i++) {
 +              struct commit *commit = todo_list.items[i].commit;
 +              if (commit)
 +                      commit->util = (void *)1;
 +      }
 +
 +      todo_list_release(&todo_list);
 +      strbuf_addstr(&todo_file, ".backup");
 +      fd = open(todo_file.buf, O_RDONLY);
 +      if (fd < 0) {
 +              res = error_errno(_("could not open '%s'"), todo_file.buf);
 +              goto leave_check;
 +      }
 +      if (strbuf_read(&todo_list.buf, fd, 0) < 0) {
 +              close(fd);
 +              res = error(_("could not read '%s'."), todo_file.buf);
 +              goto leave_check;
 +      }
 +      close(fd);
 +      strbuf_release(&todo_file);
 +      res = !!parse_insn_buffer(todo_list.buf.buf, &todo_list);
 +
 +      /* Find commits in git-rebase-todo.backup yet unseen */
 +      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) {
 +                      strbuf_addf(&missing, " - %s %.*s\n",
 +                                  short_commit_name(commit),
 +                                  item->arg_len, item->arg);
 +                      commit->util = (void *)1;
 +              }
 +      }
 +
 +      /* Warn about missing commits */
 +      if (!missing.len)
 +              goto leave_check;
 +
 +      if (check_level == CHECK_ERROR)
 +              advise_to_edit_todo = res = 1;
 +
 +      fprintf(stderr,
 +              _("Warning: some commits may have been dropped accidentally.\n"
 +              "Dropped commits (newer to older):\n"));
 +
 +      /* Make the list user-friendly and display */
 +      fputs(missing.buf, stderr);
 +      strbuf_release(&missing);
 +
 +      fprintf(stderr, _("To avoid this message, use \"drop\" to "
 +              "explicitly remove a commit.\n\n"
 +              "Use 'git config rebase.missingCommitsCheck' to change "
 +              "the level of warnings.\n"
 +              "The possible behaviours are: ignore, warn, error.\n\n"));
 +
 +leave_check:
 +      strbuf_release(&todo_file);
 +      todo_list_release(&todo_list);
 +
 +      if (advise_to_edit_todo)
 +              fprintf(stderr,
 +                      _("You can fix this with 'git rebase --edit-todo' "
 +                        "and then run 'git rebase --continue'.\n"
 +                        "Or you can abort the rebase with 'git rebase"
 +                        " --abort'.\n"));
 +
 +      return res;
 +}
 +
 +/* skip picking commits whose parents are unchanged */
 +int skip_unnecessary_picks(void)
 +{
 +      const char *todo_file = rebase_path_todo();
 +      struct strbuf buf = STRBUF_INIT;
 +      struct todo_list todo_list = TODO_LIST_INIT;
 +      struct object_id onto_oid, *oid = &onto_oid, *parent_oid;
 +      int fd, i;
 +
 +      if (!read_oneliner(&buf, rebase_path_onto(), 0))
 +              return error(_("could not read 'onto'"));
 +      if (get_oid(buf.buf, &onto_oid)) {
 +              strbuf_release(&buf);
 +              return error(_("need a HEAD to fixup"));
 +      }
 +      strbuf_release(&buf);
 +
 +      fd = open(todo_file, O_RDONLY);
 +      if (fd < 0) {
 +              return error_errno(_("could not open '%s'"), todo_file);
 +      }
 +      if (strbuf_read(&todo_list.buf, fd, 0) < 0) {
 +              close(fd);
 +              return error(_("could not read '%s'."), todo_file);
 +      }
 +      close(fd);
 +      if (parse_insn_buffer(todo_list.buf.buf, &todo_list) < 0) {
 +              todo_list_release(&todo_list);
 +              return -1;
 +      }
 +
 +      for (i = 0; i < todo_list.nr; i++) {
 +              struct todo_item *item = todo_list.items + i;
 +
 +              if (item->command >= TODO_NOOP)
 +                      continue;
 +              if (item->command != TODO_PICK)
 +                      break;
 +              if (parse_commit(item->commit)) {
 +                      todo_list_release(&todo_list);
 +                      return error(_("could not parse commit '%s'"),
 +                              oid_to_hex(&item->commit->object.oid));
 +              }
 +              if (!item->commit->parents)
 +                      break; /* root commit */
 +              if (item->commit->parents->next)
 +                      break; /* merge commit */
 +              parent_oid = &item->commit->parents->item->object.oid;
 +              if (hashcmp(parent_oid->hash, oid->hash))
 +                      break;
 +              oid = &item->commit->object.oid;
 +      }
 +      if (i > 0) {
 +              int offset = i < todo_list.nr ?
 +                      todo_list.items[i].offset_in_buf : todo_list.buf.len;
 +              const char *done_path = rebase_path_done();
 +
 +              fd = open(done_path, O_CREAT | O_WRONLY | O_APPEND, 0666);
 +              if (fd < 0) {
 +                      error_errno(_("could not open '%s' for writing"),
 +                                  done_path);
 +                      todo_list_release(&todo_list);
 +                      return -1;
 +              }
 +              if (write_in_full(fd, todo_list.buf.buf, offset) < 0) {
 +                      error_errno(_("could not write to '%s'"), done_path);
 +                      todo_list_release(&todo_list);
 +                      close(fd);
 +                      return -1;
 +              }
 +              close(fd);
 +
 +              fd = open(rebase_path_todo(), O_WRONLY, 0666);
 +              if (fd < 0) {
 +                      error_errno(_("could not open '%s' for writing"),
 +                                  rebase_path_todo());
 +                      todo_list_release(&todo_list);
 +                      return -1;
 +              }
 +              if (write_in_full(fd, todo_list.buf.buf + offset,
 +                              todo_list.buf.len - offset) < 0) {
 +                      error_errno(_("could not write to '%s'"),
 +                                  rebase_path_todo());
 +                      close(fd);
 +                      todo_list_release(&todo_list);
 +                      return -1;
 +              }
 +              if (ftruncate(fd, todo_list.buf.len - offset) < 0) {
 +                      error_errno(_("could not truncate '%s'"),
 +                                  rebase_path_todo());
 +                      todo_list_release(&todo_list);
 +                      close(fd);
 +                      return -1;
 +              }
 +              close(fd);
 +
 +              todo_list.current = i;
 +              if (is_fixup(peek_command(&todo_list, 0)))
 +                      record_in_rewritten(oid, peek_command(&todo_list, 0));
 +      }
 +
 +      todo_list_release(&todo_list);
 +      printf("%s\n", oid_to_hex(oid));
 +
 +      return 0;
 +}
 +
 +struct subject2item_entry {
 +      struct hashmap_entry entry;
 +      int i;
 +      char subject[FLEX_ARRAY];
 +};
 +
 +static int subject2item_cmp(const void *fndata,
 +                          const struct subject2item_entry *a,
 +                          const struct subject2item_entry *b, const void *key)
 +{
 +      return key ? strcmp(a->subject, key) : strcmp(a->subject, b->subject);
 +}
 +
 +/*
 + * 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
 + * after the former, and change "pick" to "fixup"/"squash".
 + *
 + * Note that if the config has specified a custom instruction format, each log
 + * message will have to be retrieved from the commit (as the oneline in the
 + * script cannot be trusted) in order to normalize the autosquash arrangement.
 + */
 +int rearrange_squash(void)
 +{
 +      const char *todo_file = rebase_path_todo();
 +      struct todo_list todo_list = TODO_LIST_INIT;
 +      struct hashmap subject2item;
 +      int res = 0, rearranged = 0, *next, *tail, fd, i;
 +      char **subjects;
 +
 +      fd = open(todo_file, O_RDONLY);
 +      if (fd < 0)
 +              return error_errno(_("could not open '%s'"), todo_file);
 +      if (strbuf_read(&todo_list.buf, fd, 0) < 0) {
 +              close(fd);
 +              return error(_("could not read '%s'."), todo_file);
 +      }
 +      close(fd);
 +      if (parse_insn_buffer(todo_list.buf.buf, &todo_list) < 0) {
 +              todo_list_release(&todo_list);
 +              return -1;
 +      }
 +
 +      /*
 +       * The hashmap maps onelines to the respective todo list index.
 +       *
 +       * If any items need to be rearranged, the next[i] value will indicate
 +       * which item was moved directly after the i'th.
 +       *
 +       * In that case, last[i] will indicate the index of the latest item to
 +       * be moved to appear after the i'th.
 +       */
 +      hashmap_init(&subject2item, (hashmap_cmp_fn) subject2item_cmp,
 +                   NULL, todo_list.nr);
 +      ALLOC_ARRAY(next, todo_list.nr);
 +      ALLOC_ARRAY(tail, todo_list.nr);
 +      ALLOC_ARRAY(subjects, todo_list.nr);
 +      for (i = 0; i < todo_list.nr; i++) {
 +              struct strbuf buf = STRBUF_INIT;
 +              struct todo_item *item = todo_list.items + i;
 +              const char *commit_buffer, *subject, *p;
 +              size_t subject_len;
 +              int i2 = -1;
 +              struct subject2item_entry *entry;
 +
 +              next[i] = tail[i] = -1;
 +              if (item->command >= TODO_EXEC) {
 +                      subjects[i] = NULL;
 +                      continue;
 +              }
 +
 +              if (is_fixup(item->command)) {
 +                      todo_list_release(&todo_list);
 +                      return error(_("the script was already rearranged."));
 +              }
 +
 +              item->commit->util = item;
 +
 +              parse_commit(item->commit);
 +              commit_buffer = get_commit_buffer(item->commit, NULL);
 +              find_commit_subject(commit_buffer, &subject);
 +              format_subject(&buf, subject, " ");
 +              subject = subjects[i] = strbuf_detach(&buf, &subject_len);
 +              unuse_commit_buffer(item->commit, commit_buffer);
 +              if ((skip_prefix(subject, "fixup! ", &p) ||
 +                   skip_prefix(subject, "squash! ", &p))) {
 +                      struct commit *commit2;
 +
 +                      for (;;) {
 +                              while (isspace(*p))
 +                                      p++;
 +                              if (!skip_prefix(p, "fixup! ", &p) &&
 +                                  !skip_prefix(p, "squash! ", &p))
 +                                      break;
 +                      }
 +
 +                      if ((entry = hashmap_get_from_hash(&subject2item,
 +                                                         strhash(p), p)))
 +                              /* found by title */
 +                              i2 = entry->i;
 +                      else if (!strchr(p, ' ') &&
 +                               (commit2 =
 +                                lookup_commit_reference_by_name(p)) &&
 +                               commit2->util)
 +                              /* found by commit name */
 +                              i2 = (struct todo_item *)commit2->util
 +                                      - todo_list.items;
 +                      else {
 +                              /* copy can be a prefix of the commit subject */
 +                              for (i2 = 0; i2 < i; i2++)
 +                                      if (subjects[i2] &&
 +                                          starts_with(subjects[i2], p))
 +                                              break;
 +                              if (i2 == i)
 +                                      i2 = -1;
 +                      }
 +              }
 +              if (i2 >= 0) {
 +                      rearranged = 1;
 +                      todo_list.items[i].command =
 +                              starts_with(subject, "fixup!") ?
 +                              TODO_FIXUP : TODO_SQUASH;
 +                      if (next[i2] < 0)
 +                              next[i2] = i;
 +                      else
 +                              next[tail[i2]] = i;
 +                      tail[i2] = i;
 +              } else if (!hashmap_get_from_hash(&subject2item,
 +                                              strhash(subject), subject)) {
 +                      FLEX_ALLOC_MEM(entry, subject, subject, subject_len);
 +                      entry->i = i;
 +                      hashmap_entry_init(entry, strhash(entry->subject));
 +                      hashmap_put(&subject2item, entry);
 +              }
 +      }
 +
 +      if (rearranged) {
 +              struct strbuf buf = STRBUF_INIT;
 +
 +              for (i = 0; i < todo_list.nr; i++) {
 +                      enum todo_command command = todo_list.items[i].command;
 +                      int cur = i;
 +
 +                      /*
 +                       * Initially, all commands are 'pick's. If it is a
 +                       * fixup or a squash now, we have rearranged it.
 +                       */
 +                      if (is_fixup(command))
 +                              continue;
 +
 +                      while (cur >= 0) {
 +                              int offset = todo_list.items[cur].offset_in_buf;
 +                              int end_offset = cur + 1 < todo_list.nr ?
 +                                      todo_list.items[cur + 1].offset_in_buf :
 +                                      todo_list.buf.len;
 +                              char *bol = todo_list.buf.buf + offset;
 +                              char *eol = todo_list.buf.buf + end_offset;
 +
 +                              /* replace 'pick', by 'fixup' or 'squash' */
 +                              command = todo_list.items[cur].command;
 +                              if (is_fixup(command)) {
 +                                      strbuf_addstr(&buf,
 +                                              todo_command_info[command].str);
 +                                      bol += strcspn(bol, " \t");
 +                              }
 +
 +                              strbuf_add(&buf, bol, eol - bol);
 +
 +                              cur = next[cur];
 +                      }
 +              }
 +
 +              fd = open(todo_file, O_WRONLY);
 +              if (fd < 0)
 +                      res = error_errno(_("could not open '%s'"), todo_file);
 +              else if (write(fd, buf.buf, buf.len) < 0)
 +                      res = error_errno(_("could not write to '%s'"), todo_file);
 +              else if (ftruncate(fd, buf.len) < 0)
 +                      res = error_errno(_("could not truncate '%s'"),
 +                                         todo_file);
 +              close(fd);
 +              strbuf_release(&buf);
 +      }
 +
 +      free(next);
 +      free(tail);
 +      for (i = 0; i < todo_list.nr; i++)
 +              free(subjects[i]);
 +      free(subjects);
 +      hashmap_free(&subject2item, 1);
 +      todo_list_release(&todo_list);
 +
 +      return res;
 +}
diff --combined sha1_file.c
index 10c3a0083dedee98dc3d48267ad349589adef3f0,1d1747099a31fe20f57e26fc85ec65578a01a2d1..00f5b9e0b2ddd93f1e91e2e74c984c7a76f9c11c
@@@ -456,19 -456,19 +456,19 @@@ struct alternate_object_database *alloc
  
  void add_to_alternates_file(const char *reference)
  {
-       struct lock_file *lock = xcalloc(1, sizeof(struct lock_file));
+       struct lock_file lock = LOCK_INIT;
        char *alts = git_pathdup("objects/info/alternates");
        FILE *in, *out;
+       int found = 0;
  
-       hold_lock_file_for_update(lock, alts, LOCK_DIE_ON_ERROR);
-       out = fdopen_lock_file(lock, "w");
+       hold_lock_file_for_update(&lock, alts, LOCK_DIE_ON_ERROR);
+       out = fdopen_lock_file(&lock, "w");
        if (!out)
                die_errno("unable to fdopen alternates lockfile");
  
        in = fopen(alts, "r");
        if (in) {
                struct strbuf line = STRBUF_INIT;
-               int found = 0;
  
                while (strbuf_getline(&line, in) != EOF) {
                        if (!strcmp(reference, line.buf)) {
  
                strbuf_release(&line);
                fclose(in);
-               if (found) {
-                       rollback_lock_file(lock);
-                       lock = NULL;
-               }
        }
        else if (errno != ENOENT)
                die_errno("unable to read alternates file");
  
-       if (lock) {
+       if (found) {
+               rollback_lock_file(&lock);
+       } else {
                fprintf_or_die(out, "%s\n", reference);
-               if (commit_lock_file(lock))
+               if (commit_lock_file(&lock))
                        die_errno("unable to move new alternates file into place");
                if (alt_odb_tail)
                        link_alt_odb_entries(reference, '\n', NULL, 0);
@@@ -1124,14 -1121,10 +1121,14 @@@ static int sha1_loose_object_info(cons
        } else if ((status = parse_sha1_header_extended(hdr, oi, flags)) < 0)
                status = error("unable to parse %s header", sha1_to_hex(sha1));
  
 -      if (status >= 0 && oi->contentp)
 +      if (status >= 0 && oi->contentp) {
                *oi->contentp = unpack_sha1_rest(&stream, hdr,
                                                 *oi->sizep, sha1);
 -      else
 +              if (!*oi->contentp) {
 +                      git_inflate_end(&stream);
 +                      status = -1;
 +              }
 +      } else
                git_inflate_end(&stream);
  
        munmap(map, mapsize);
@@@ -1752,15 -1745,10 +1749,15 @@@ static int index_core(unsigned char *sh
                ret = index_mem(sha1, "", size, type, path, flags);
        } else if (size <= SMALL_FILE_SIZE) {
                char *buf = xmalloc(size);
 -              if (size == read_in_full(fd, buf, size))
 -                      ret = index_mem(sha1, buf, size, type, path, flags);
 +              ssize_t read_result = read_in_full(fd, buf, size);
 +              if (read_result < 0)
 +                      ret = error_errno("read error while indexing %s",
 +                                        path ? path : "<unknown>");
 +              else if (read_result != size)
 +                      ret = error("short read while indexing %s",
 +                                  path ? path : "<unknown>");
                else
 -                      ret = error_errno("short read");
 +                      ret = index_mem(sha1, buf, size, type, path, flags);
                free(buf);
        } else {
                void *buf = xmmap(NULL, size, PROT_READ, MAP_PRIVATE, fd, 0);
diff --combined wt-status.c
index 29bc64cc0280675065142bac257930904216d415,7d88e7ca8539df06d3f2fb30a1bbcf3d36145d95..93ac645225e6d6ce3000ff866015806ce9630383
@@@ -121,13 -121,15 +121,13 @@@ static void status_printf_more(struct w
  
  void wt_status_prepare(struct wt_status *s)
  {
 -      struct object_id oid;
 -
        memset(s, 0, sizeof(*s));
        memcpy(s->color_palette, default_wt_status_colors,
               sizeof(default_wt_status_colors));
        s->show_untracked_files = SHOW_NORMAL_UNTRACKED_FILES;
        s->use_color = -1;
        s->relative_paths = 1;
 -      s->branch = resolve_refdup("HEAD", 0, oid.hash, NULL);
 +      s->branch = resolve_refdup("HEAD", 0, NULL, NULL);
        s->reference = "HEAD";
        s->fp = stdout;
        s->index_file = get_index_file();
@@@ -2297,14 -2299,14 +2297,14 @@@ int has_uncommitted_changes(int ignore_
   */
  int require_clean_work_tree(const char *action, const char *hint, int ignore_submodules, int gently)
  {
-       struct lock_file *lock_file = xcalloc(1, sizeof(*lock_file));
+       struct lock_file lock_file = LOCK_INIT;
        int err = 0, fd;
  
-       fd = hold_locked_index(lock_file, 0);
+       fd = hold_locked_index(&lock_file, 0);
        refresh_cache(REFRESH_QUIET);
        if (0 <= fd)
-               update_index_if_able(&the_index, lock_file);
-       rollback_lock_file(lock_file);
+               update_index_if_able(&the_index, &lock_file);
+       rollback_lock_file(&lock_file);
  
        if (has_unstaged_changes(ignore_submodules)) {
                /* TRANSLATORS: the action is e.g. "pull with rebase" */