Re: [PATCH 00/12] Hard coded string length cleanup
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Dec 18, 2013, 18:06 UTC
- Message-ID
- <xmqqtxe6hw4t.fsf@gitster.dls.corp.google.com>
- In-Reply-To
- <1387378437-20646-1-git-send-email-pclouds@gmail.com>
Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:
Show 10 quoted lines
> 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.
Show 60 quoted lines
> > 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(-)