Web lists-archives.com

[PATCH 00/11] various lockfile-leaks and -fixes




A recent series allowed `struct lock_file`s to be freed [1], so I wanted
to get rid of the "simple" leaks of this kind. I found a couple of
lock-related cleanups along the way and it resulted in this series. It
feels a bit scary at eleven patches -- especially as this is about
locking -- but I hope I've managed to put this into understandable and
reviewable patches. Reviews, thoughts, opinions welcome, as always.

1-2

Instead of allocating and leaking `struct lock_file`s, initialize them
on the stack. These instances were identified by simple grepping.

3-4

Documentation fixes for lockfile and temporary file APIs.

5-7

No need to represent the same information twice. No need for the caller
to provide a (static) lock for us now that we can safely manage our own.

8-11

Error-handling in read-cache.c was a bit lacking and over-eager at the
same time. Sometimes we'd leave the lock open when we should commit it,
sometimes we'd roll it back when we should only close it. We'd also
crash under rare circumstances. Patch 9 addresses a bug in the API which
most likely hasn't hit anyone and maybe never would, but if we don't
have the functionality, I don't think we should pretend like we do.

Martin

[1] https://public-inbox.org/git/20170905121353.62zg3mtextmq5zrs@xxxxxxxxxxxxxxxxxxxxx/

Martin Ågren (11):
  sha1_file: do not leak `lock_file`
  treewide: prefer lockfiles on the stack
  lockfile: fix documentation on `close_lock_file_gently()`
  tempfile: fix documentation on `delete_tempfile()`
  cache-tree: simplify locking logic
  apply: move lockfile into `apply_state`
  apply: remove `newfd` from `struct apply_state`
  cache.h: document `write_locked_index()`
  read-cache: require flags for `write_locked_index()`
  read-cache: don't leave dangling pointer in `do_write_index()`
  read-cache: roll back lock on error with `COMMIT_LOCK`

 apply.c            | 25 ++++++++-----------------
 apply.h            |  8 +++-----
 builtin/am.c       | 27 ++++++++++++---------------
 builtin/apply.c    |  4 +---
 builtin/checkout.c | 14 ++++++--------
 builtin/clone.c    |  7 +++----
 builtin/diff.c     |  7 +++----
 builtin/difftool.c |  1 -
 cache-tree.c       | 12 ++++--------
 cache.h            | 19 +++++++++++++++++++
 config.c           | 17 ++++++++---------
 git-compat-util.h  |  7 ++++++-
 lockfile.h         |  4 ++--
 merge-recursive.c  |  6 +++---
 merge.c            |  8 +++-----
 read-cache.c       | 26 ++++++++++++++------------
 sequencer.c        |  1 -
 sha1_file.c        | 16 +++++++---------
 tempfile.h         |  8 ++++----
 wt-status.c        |  8 ++++----
 20 files changed, 110 insertions(+), 115 deletions(-)

-- 
2.14.1.727.g9ddaf86