[PATCH v2 0/68] war on sprintf

This is a revised version of the series I sent earlier[1].

For those just joining us, the goal is to remove sprintf, strcpy, etc, to make it easier to audit the code base for buffer overflows. I've been addressing review comments for individual patches as the discussion progressed, so there's nothing here that hasn't been on the list (and I think I've addressed all of the comments from the first round of review).

Here's a list of the commits, with an overview[2] of the changes from v1 (patches without comment are the same as in v1):

  [01/68]: show-branch: avoid segfault with --reflog of unborn branch
  [02/68]: mailsplit: fix FILE* leak in split_maildir
  [03/68]: archive-tar: fix minor indentation violation
  [04/68]: fsck: don't fsck alternates for connectivity-only check
  [05/68]: add xsnprintf helper function
  [06/68]: add git_path_buf helper function
  [07/68]: strbuf: make strbuf_complete_line more generic

    Docstring now makes clear the behavior when the strbuf is empty.

  [08/68]: add reentrant variants of sha1_to_hex and find_unique_abbrev

    These are now called sha1_to_hex_r, etc. The re-entrancy conditions
    are documented.

  [09/68]: fsck: use strbuf to generate alternate directories
  [10/68]: mailsplit: make PATH_MAX buffers dynamic

    Use xstrfmt consistently instead of sometimes using a strbuf.

  [11/68]: trace: use strbuf for quote_crnl output

    Further cleanup of "p2" variable in quote_crnl.

  [12/68]: progress: store throughput display in a strbuf
  [13/68]: test-dump-cache-tree: avoid overflow of cache-tree name
  [14/68]: compat/inet_ntop: fix off-by-one in inet_ntop4
  [15/68]: convert trivial sprintf / strcpy calls to xsnprintf

    Prefer "fixed-size" to "static" for clarity in commit message.

  [16/68]: archive-tar: use xsnprintf for trivial formatting
  [17/68]: use xsnprintf for generating git object headers
  [18/68]: find_short_object_filename: convert sprintf to xsnprintf
  [19/68]: stop_progress_msg: convert sprintf to xsnprintf

    Ditto on "fixed-size" versus "static".

  [20/68]: compat/hstrerror: convert sprintf to snprintf
  [21/68]: grep: use xsnprintf to format failure message
  [22/68]: entry.c: convert strcpy to xsnprintf
  [23/68]: add_packed_git: convert strcpy into xsnprintf

    Drop useless comment. Add comment on magic strlen().

  [24/68]: http-push: replace strcat with xsnprintf
  [25/68]: receive-pack: convert strncpy to xsnprintf
  [26/68]: replace trivial malloc + sprintf / strcpy calls with xstrfmt

    Include NUL in base64 of imap-send cram response. My guess is this
    is a bug in what we send that is overlooked by most servers, but I'd
    prefer to be conservative here and keep the behavior the same.

  [27/68]: config: use xstrfmt in normalize_value
  [28/68]: fetch: replace static buffer with xstrfmt
  [29/68]: use strip_suffix and xstrfmt to replace suffix
  [30/68]: ref-filter: drop sprintf and strcpy calls
  [31/68]: help: drop prepend function in favor of xstrfmt
  [32/68]: mailmap: replace strcpy with xstrdup
  [33/68]: read_branches_file: simplify string handling

    This goes much further than the original in cleaning up the use of a
    static buffer. From what I sent earlier during review, I dropped
    strbuf_read_file in favor of strbuf_getline, to keep the behavior
    identical to the original.

  [34/68]: read_remotes_file: simplify string handling

    New in this iteration; cleanups to match those in 33/68.

  [35/68]: resolve_ref: use strbufs for internal buffers
  [36/68]: upload-archive: convert sprintf to strbuf
  [37/68]: remote-ext: simplify git pkt-line generation

    The v1 of this patch was totally buggy. This is a rewrite to use
    packet_write(), which has several advantages, and includes tests.

  [38/68]: http-push: use strbuf instead of fwrite_buffer
  [39/68]: http-walker: store url in a strbuf
  [40/68]: sha1_get_pack_name: use a strbuf
  [41/68]: init: use strbufs to store paths
  [42/68]: apply: convert root string to strbuf
  [43/68]: transport: use strbufs for status table "quickref" strings
  [44/68]: merge-recursive: convert malloc / strcpy to strbuf
  [45/68]: enter_repo: convert fixed-size buffers to strbufs
  [46/68]: remove_leading_path: use a strbuf for internal storage
  [47/68]: write_loose_object: convert to strbuf

    Clarify comment on subtle mkstemp behavior.

  [48/68]: diagnose_invalid_index_path: use strbuf to avoid strcpy/strcat
  [49/68]: fetch-pack: use argv_array for index-pack / unpack-objects
  [50/68]: http-push: use an argv_array for setup_revisions
  [51/68]: stat_tracking_info: convert to argv_array
  [52/68]: daemon: use cld->env_array when re-spawning
  [53/68]: use sha1_to_hex_r() instead of strcpy

    Use "_r" versions to match change in patch 8.

  [54/68]: drop strcpy in favor of raw sha1_to_hex

    More history in the commit message, courtesy of Eric.

  [55/68]: color: add overflow checks for parsing colors

    Be more careful with OUT() macro.

  [56/68]: use alloc_ref rather than hand-allocating "struct ref"
  [57/68]: avoid sprintf and strcpy with flex arrays

    Trivial typo-fix in comment.

  [58/68]: receive-pack: simplify keep_arg computation

    Another s/static/fixed-size/.

  [59/68]: help: clean up kfmclient munging
  [60/68]: prefer memcpy to strcpy
  [61/68]: color: add color_set helper for copying raw colors
  [62/68]: notes: document length of fanout path with a constant
  [63/68]: convert strncpy to memcpy
  [64/68]: fsck: drop inode-sorting code
  [65/68]: Makefile: drop D_INO_IN_DIRENT build knob
  [66/68]: fsck: use for_each_loose_file_in_objdir
  [67/68]: use strbuf_complete to conditionally append slash

    Drop bogus conversion from imap-send.

  [68/68]: name-rev: use strip_suffix to avoid magic numbers

Hopefully that makes reviewing v2 a little less painful for those who already slogged through v1. Thanks for your patience. :)

-Peff

[1] Gmane seems down, but once it's back up you should be able to get to
    v1 at: http://mid.gmane.org/20150915152125.GA27504@sigill.intra.peff.net

[2] Keeping track of the tweaks to this many patches would have been
    near impossible without Thomas Rast's git-tbdiff tool:

      https://github.com/trast/tbdiff

    I highly recommend it for people handling multiple versions of a
    long series.