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