public inbox for git@vger.kernel.org 
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox•com>
To: "Nguyễn Thái Ngọc Duy" <pclouds@gmail•com>
Cc: git@vger•kernel.org
Subject: Re: [PATCH 00/12] Hard coded string length cleanup
Date: Wed, 18 Dec 2013 10:06:10 -0800	[thread overview]
Message-ID: <xmqqtxe6hw4t.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <1387378437-20646-1-git-send-email-pclouds@gmail.com> ("Nguyễn	Thái Ngọc Duy"'s message of "Wed, 18 Dec 2013 21:53:45 +0700")

Nguyễn Thái Ngọc Duy  <pclouds@gmail•com> 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(-)

  parent reply	other threads:[~2013-12-18 18:06 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-12-18 14:53 [PATCH 00/12] Hard coded string length cleanup Nguyễn Thái Ngọc Duy
2013-12-18 14:53 ` [PATCH 01/12] Make starts_with() a wrapper of skip_prefix() Nguyễn Thái Ngọc Duy
2013-12-18 17:50   ` Junio C Hamano
2013-12-18 18:16     ` Junio C Hamano
2013-12-18 14:53 ` [PATCH 02/12] Convert starts_with() to skip_prefix() for option parsing Nguyễn Thái Ngọc Duy
2013-12-20  6:51   ` Johannes Sixt
2013-12-20  7:04     ` Jeff King
2013-12-20  8:46       ` Christian Couder
2013-12-20 10:43       ` René Scharfe
2013-12-20 21:31       ` Junio C Hamano
2013-12-21  4:44         ` Duy Nguyen
2013-12-26 19:27           ` Junio C Hamano
2013-12-28  9:54             ` Jeff King
2013-12-18 14:53 ` [PATCH 03/12] Add and use skip_prefix_defval() Nguyễn Thái Ngọc Duy
2013-12-18 16:27   ` Kent R. Spillner
2013-12-18 17:51     ` Junio C Hamano
2013-12-18 14:53 ` [PATCH 04/12] Replace some use of starts_with() with skip_prefix() Nguyễn Thái Ngọc Duy
2013-12-18 14:53 ` [PATCH 05/12] Convert a lot of starts_with() to skip_prefix() Nguyễn Thái Ngọc Duy
2013-12-18 14:53 ` [PATCH 06/12] fetch.c: replace some use of starts_with() with skip_prefix() Nguyễn Thái Ngọc Duy
2013-12-18 14:53 ` [PATCH 07/12] connect.c: " Nguyễn Thái Ngọc Duy
2013-12-18 14:53 ` [PATCH 08/12] refs.c: " Nguyễn Thái Ngọc Duy
2013-12-18 14:53 ` [PATCH 09/12] diff.c: reduce code duplication in --stat-xxx parsing Nguyễn Thái Ngọc Duy
2013-12-18 14:53 ` [PATCH 10/12] environment.c: replace starts_with() in strip_namespace() with skip_prefix() Nguyễn Thái Ngọc Duy
2013-12-18 14:53 ` [PATCH 11/12] diff.c: convert diff_scoreopt_parse to use skip_prefix() Nguyễn Thái Ngọc Duy
2013-12-18 14:53 ` [PATCH 12/12] refs.c: use skip_prefix() in prune_ref() Nguyễn Thái Ngọc Duy
2013-12-18 18:06 ` Junio C Hamano [this message]
2013-12-19 23:32 ` [PATCH 00/12] Hard coded string length cleanup René Scharfe
2013-12-19 23:50   ` Duy Nguyen
2013-12-20  1:06     ` René Scharfe
2013-12-20  2:29       ` Duy Nguyen
2013-12-20 16:53       ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqtxe6hw4t.fsf@gitster.dls.corp.google.com \
    --to=gitster@pobox$(echo .)com \
    --cc=git@vger$(echo .)kernel.org \
    --cc=pclouds@gmail$(echo .)com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox