[PATCH 0/8] write_file cleanups

From: Jeff King <peff@peff.net>
Date: Jul 8, 2016, 09:04 UTC
Message-ID: <20160708090400.GA26594@sigill.intra.peff.net>
In-Reply-To: <20160708065650.GA10664@sigill.intra.peff.net>

On Fri, Jul 08, 2016 at 02:56:50AM -0400, Jeff King wrote:

Show 18 quoted lines

> > This is more an illustration of unnecessarily duplicated code, isn't it?
> > There are *tons* of instances in Git's code where writing to a file is
> > implemented separately (and differently).
> > 
> > It would make tons of sense to consolidate all of these instances,
> > methinks. The diffstat should look *very* pleasing.
> 
> I grepped for O_WRONLY, and there are fewer instances than I would have
> thought. Most of the obvious write_file() candidates are in the merge
> code, which is probably why you saw so many of them. :)
> 
> I started at converting a few sites, but it's actually a little awkward
> because they all have strbufs (with a ptr/len combo that _could_ contain
> NULs, but probably doesn't), and write_file() wants to take a format
> string.
> 
> I think we can clean that up, though. I'll hopefully have a series in a
> few minutes.

Here it is. There actually weren't that many spots to clean up, as quite a few of them have a "twist" where they want to do something clever, like open the file and feed the descriptor to a sub-function, or open with funny things like O_EXCL.

But still, the diffstat is pleasing:

 builtin/am.c     | 25 +++++++----------
 builtin/branch.c |  5 +---
 builtin/config.c |  2 +-
 builtin/merge.c  | 45 ++++--------------------------
 cache.h          | 17 ++++++++++--
 wrapper.c        | 52 ++++++++---------------------------
 6 files changed, 44 insertions(+), 102 deletions(-)

and that even includes adding some function documentation.

The most interesting thing is that I also found a real bug, albeit a fairly minor one. I floated that up to the front of the series.

  [1/8]: config: fix bogus fd check when setting up default config
  [2/8]: am: ignore return value of write_file()
  [3/8]: branch: use non-gentle write_file for branch description
  [4/8]: write_file: drop "gently" form
  [5/8]: write_file: use xopen
  [6/8]: write_file: add pointer+len variant
  [7/8]: write_file: add format attribute
  [8/8]: use write_file_buf where applicable

-Peff