Re: [PATCH] archive-tar: fix sanity check in config parsing

[Jeff King <peff@peff.net>]

On Mon, Jan 14, 2013 at 04:44:24AM -0800, Jeff King wrote:

> > Wouldn't it then be better ti use strlen("tar") rather than a 3? Or
> > at least a comment?
> [...]
> We could also potentially encapsulate it in a function. I think the diff
> code has a very similar block.

Here's a series that does that, with a few other cleanups on top. The
diffstat actually ends up a few lines longer, but that is mostly because
of comments and function declarations. More importantly, though, the
call-sites are much easier to read.

Having written this series, though, I can't help but wonder if the world
would be a better place if config_fn_t looked more like:

  typedef int (*config_fn_t)(const char *full_var,
                             const char *section,
                             const char *subsection,
                             const char *key,
                             const char *value,
                             void *data);

It's just as easy for the config parser to do this ahead of time, and by
handing off real C-strings (instead of ending up with a ptr/len pair for
the subsection), it makes the lives of the callbacks much easier (e.g.,
the final patch below contorts a bit to use string_list with the
subsection).

I can look into that, but here is the less invasive cleanup:

  [1/6]: config: add helper function for parsing key names
  [2/6]: archive-tar: use match_config_key when parsing config
  [3/6]: convert some config callbacks to match_config_key
  [4/6]: userdiff: drop parse_driver function
  [5/6]: submodule: use match_config_key when parsing config
  [6/6]: submodule: simplify memory handling in config parsing

-Peff