[PATCH 0/14] Re: copy.c: make copy_fd preserve meaningful errno

From: Jonathan Nieder <jrnieder@gmail.com>
Date: Dec 3, 2014, 05:02 UTC
Message-ID: <20141203050217.GJ6527@google.com>
In-Reply-To: <CAGZ79kbF6JjxgHX2KZFhSh9QyGOXeS=cVK0z=CM4n9-ErRDJ8A@mail.gmail.com>

Stefan Beller wrote:

> On Mon, Nov 17, 2014 at 4:48 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:

Show 6 quoted lines

>> After this patch, setting errno is not part of the contract of
>> copy_fd, so the bug Ronnie was fixing is gone.
>>
>> But it's a little more invasive.  What do you think?
>
> I really like that approach and would be happy if

Thanks for looking it over, and sorry for the delay. Here's a series that carries out that approach.

The patch I'm least happy with in this series is 12/14, mostly because it's just so noisy. It would be safe (and non-leaky) to leave out most of those strbuf_release calls since nobody appends to 'err' in the non-error case, but always free-ing means that normal escape analysis can work. I could be convinced to go either way.

Jonathan Nieder (14):
  strbuf: introduce strbuf_prefixf()
  add_to_alternates_file: respect GIT_OBJECT_DIRECTORY
  copy_fd: pass error message back through a strbuf
  hold_lock_file_for_append: pass error message back through a strbuf
  lock_packed_refs: pass error message back through a strbuf
  lockfile: introduce flag for locks outside .git
  fast-import: use message from lockfile API when writing marks fails
  credentials: use message from lockfile API when locking
    ~/.git-credentials fails
  config: use message from lockfile API when locking config file fails
  rerere: error out on autoupdate failure
  hold_locked_index: pass error message back through a strbuf
  hold_lock_file_for_update: pass error message back through a strbuf
  lockfile: remove unused function 'unable_to_lock_die'
  lockfile: make 'unable_to_lock_message' private

 Documentation/technical/api-lockfile.txt | 41 ++++++---------
 builtin/add.c                            | 12 +++--
 builtin/apply.c                          | 15 ++++--
 builtin/checkout-index.c                 | 10 +++-
 builtin/checkout.c                       | 55 ++++++++++++++------
 builtin/clone.c                          | 12 ++++-
 builtin/commit.c                         | 36 +++++++++----
 builtin/describe.c                       | 11 ++--
 builtin/diff.c                           | 12 +++--
 builtin/gc.c                             |  8 ++-
 builtin/merge.c                          | 14 +++--
 builtin/mv.c                             |  5 +-
 builtin/read-tree.c                      |  9 +++-
 builtin/reset.c                          | 10 +++-
 builtin/rm.c                             |  9 +++-
 builtin/update-index.c                   | 10 ++--
 bundle.c                                 | 10 ++--
 cache-tree.c                             | 18 +++++--
 cache.h                                  |  4 +-
 config.c                                 | 14 +++--
 convert.c                                |  6 ++-
 copy.c                                   | 32 ++++++++----
 credential-store.c                       |  8 ++-
 fast-import.c                            |  9 ++--
 lockfile.c                               | 89 ++++++++++++++------------------
 lockfile.h                               | 13 +++--
 merge-recursive.c                        | 13 +++--
 merge.c                                  | 17 ++++--
 read-cache.c                             |  7 +--
 refs.c                                   | 28 ++++++----
 refs.h                                   |  8 +--
 rerere.c                                 | 24 +++++----
 sequencer.c                              | 33 +++++++++---
 sha1_file.c                              | 17 ++++--
 shallow.c                                | 16 ++++--
 strbuf.c                                 | 16 ++++++
 strbuf.h                                 |  4 ++
 test-scrap-cache-tree.c                  |  5 +-
 38 files changed, 434 insertions(+), 226 deletions(-)