Subject: Re: [PATCH 00/12] Hard coded string length cleanup

From: Junio C Hamano <[email-masked]>
Date: Wed, 18 Dec 2013 10:06:10 -0800

Nguyễn Thái Ngọc Duy  <[email-masked]> writes:

> I reimplemented skip_prefix() again just to realize this function
> already exists. Which reminds me there are a bunch of places that
> could benefit from this function, the same reason that I wanted to
> reimplement it.
>
> So this is series to make it more popular (so hopefully I'll see it
> used somewhere and know that it exists) and the code cleaner. The
> pattern "compare a string, then skip the compared part by a hard coded
> string length" is almost killed. I left a few in places for those who
> want to contribute :)

Overall the goal of getting rid of the "if the first N bytes of the
ptr match this string, advance ptr by N bytes" pattern, whether the
first N is implicitly given by using prefixcmp() or explicitly given
by using memcmp(), is very good.  And the result looked good from a
cursory read---though I have not gone through the series with fine
toothed comb yet.

Thanks.

>
> Nguyễn Thái Ngọc Duy (12):
>   Make starts_with() a wrapper of skip_prefix()
>   Convert starts_with() to skip_prefix() for option parsing
>   Add and use skip_prefix_defval()
>   Replace some use of starts_with() with skip_prefix()
>   Convert a lot of starts_with() to skip_prefix()
>   fetch.c: replace some use of starts_with() with skip_prefix()
>   connect.c: replace some use of starts_with() with skip_prefix()
>   refs.c: replace some use of starts_with() with skip_prefix()
>   diff.c: reduce code duplication in --stat-xxx parsing
>   environment.c: replace starts_with() in strip_namespace() with skip_prefix()
>   diff.c: convert diff_scoreopt_parse to use skip_prefix()
>   refs.c: use skip_prefix() in prune_ref()
>
>  builtin/branch.c         |   3 +-
>  builtin/checkout.c       |   6 +-
>  builtin/fast-export.c    |   3 +-
>  builtin/fetch-pack.c     |  13 ++--
>  builtin/fetch.c          |  16 ++---
>  builtin/for-each-ref.c   |   9 +--
>  builtin/index-pack.c     |  17 +++---
>  builtin/ls-remote.c      |   9 +--
>  builtin/mailinfo.c       |  11 ++--
>  builtin/merge.c          |  12 ++--
>  builtin/reflog.c         |   9 +--
>  builtin/remote.c         |   3 +-
>  builtin/rev-parse.c      |  41 ++++++-------
>  builtin/send-pack.c      |  18 +++---
>  builtin/show-branch.c    |  14 ++---
>  builtin/unpack-objects.c |   5 +-
>  builtin/update-ref.c     |  21 +++----
>  commit.c                 |   5 +-
>  connect.c                |   6 +-
>  daemon.c                 |  75 +++++++++++------------
>  diff.c                   | 153 +++++++++++++++++++++--------------------------
>  environment.c            |   4 +-
>  fetch-pack.c             |   9 +--
>  git-compat-util.h        |  15 ++++-
>  git.c                    |  16 ++---
>  http-backend.c           |   5 +-
>  http-push.c              |   6 +-
>  http.c                   |   5 +-
>  log-tree.c               |   5 +-
>  merge-recursive.c        |  13 ++--
>  notes.c                  |   6 +-
>  pager.c                  |   2 +-
>  pathspec.c               |   5 +-
>  pretty.c                 |   3 +-
>  refs.c                   |  20 ++++---
>  revision.c               |  60 +++++++++----------
>  setup.c                  |   3 +-
>  sha1_name.c              |  12 +---
>  strbuf.c                 |   9 ---
>  tag.c                    |   7 +--
>  transport-helper.c       |  15 +++--
>  transport.c              |  14 +++--
>  upload-pack.c            |   5 +-
>  wt-status.c              |  15 ++---
>  44 files changed, 334 insertions(+), 369 deletions(-)