[PATCH 0/8] write_file cleanups

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

> > 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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]