git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Andreas Ericsson <ae@op5.se>
To: "René Scharfe" <rene.scharfe@lsrfire.ath.cx>
Cc: Junio C Hamano <gitster@pobox.com>,
	Pierre Habouzit <madcoder@debian.org>,
	Git Mailing List <git@vger.kernel.org>,
	Johannes Schindelin <Johannes.Schindelin@gmx.de>
Subject: Re: [PATCH 1/2] Add strchrnul()
Date: Fri, 09 Nov 2007 11:22:37 +0100	[thread overview]
Message-ID: <473434ED.50002@op5.se> (raw)
In-Reply-To: <4733AEA0.1060602@lsrfire.ath.cx>

René Scharfe wrote:
> As suggested by Pierre Habouzit, add strchrnul().  It's a useful GNU
> extension and can simplify string parser code.  There are several
> places in git that can be converted to strchrnul(); as a trivial
> example, this patch introduces its usage to builtin-fetch--tool.c.
> 
> Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
> ---
> 
>  Makefile              |   13 +++++++++++++
>  builtin-fetch--tool.c |    8 ++------
>  compat/strchrnul.c    |    8 ++++++++
>  git-compat-util.h     |    5 +++++
>  4 files changed, 28 insertions(+), 6 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index 0d5590f..578c999 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -30,6 +30,8 @@ all::
>  #
>  # Define NO_MEMMEM if you don't have memmem.
>  #
> +# Define NO_STRCHRNUL if you don't have strchrnul.
> +#


This seems overly complicated. How about this instead?

From: Andreas Ericsson <ae@op5.se>
Subject: [PATCH] Add strchrnul()

As suggested by Pierre Habouzit, add strchrnul().  It's a useful GNU
extension and can simplify string parser code.  There are several
places in git that can be converted to strchrnul(); as a trivial
example, this patch introduces its usage to builtin-fetch--tool.c.

strchrnul() was introduced in glibc in April 1999 and included in
glibc-2.1. Checking for that version means the majority of all git
users would get to use the optimized version in glibc. Of the
remaining few some might get to use a slightly slower version
than necessary but probably not slower than what we have today.

Original patch by Rene Scharfe <rene.scharfe@lsrfire.ath.cx>

Signed-off-by: Andreas Ericsson <ae@op5.se>
---

I'm fairly much against forcing people to know what library
functions they have in order to get software to compile
properly. This is, imo, a neater solution, and also inlines
the function as suggested by Dscho.

diff --git a/git-compat-util.h b/git-compat-util.h
index 7b29d1b..9fedf33 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -105,6 +105,18 @@ extern void set_die_routine(void (*routine)(const char *err
 extern void set_error_routine(void (*routine)(const char *err, va_list params))
 extern void set_warn_routine(void (*routine)(const char *warn, va_list params))
 
+
+#if !defined(__GLIBC__) && !__GLIBC_PREREQ(2, 1)
+# define strchrnul(s, c) gitstrchrnul(s, c)
+static inline char *gitstrchrnul(const char *s, int c)
+{
+       while (*s && *s != c)
+               s++;
+
+       return (char *)s;
+}
+#endif
+
 #ifdef NO_MMAP
 
 #ifndef PROT_READ
diff --git a/builtin-fetch--tool.c b/builtin-fetch--tool.c
index 6a78517..ed60847 100644
--- a/builtin-fetch--tool.c
+++ b/builtin-fetch--tool.c
@@ -435,9 +435,7 @@ static int pick_rref(int sha1_only, const char *rref, const char *ls_remote_resu
 				cp++;
 			if (!*cp)
 				break;
-			np = strchr(cp, '\n');
-			if (!np)
-				np = cp + strlen(cp);
+			np = strchrnul(cp, '\n');
 			if (pass) {
 				lrr_list[i].line = cp;
 				lrr_list[i].name = cp + 41;
@@ -461,9 +459,7 @@ static int pick_rref(int sha1_only, const char *rref, const char *ls_remote_resu
 			rref++;
 		if (!*rref)
 			break;
-		next = strchr(rref, '\n');
-		if (!next)
-			next = rref + strlen(rref);
+		next = strchrnul(rref, '\n');
 		rreflen = next - rref;
 
 		for (i = 0; i < lrr_count; i++) {
-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231

  parent reply	other threads:[~2007-11-09 10:23 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-11-09  0:49 [PATCH 1/2] Add strchrnul() René Scharfe
2007-11-09  1:21 ` Johannes Schindelin
2007-11-09  3:31 ` Jeff King
2007-11-09 10:22 ` Andreas Ericsson [this message]
2007-11-09 13:42   ` Jakub Narebski
2007-11-09 13:59     ` Andreas Ericsson
2007-11-09 16:44       ` Jakub Narebski
2007-11-10 11:55   ` [PATCH] Simplify strchrnul() compat code René Scharfe
2007-11-10 14:04     ` Andreas Ericsson
2007-11-11 10:44       ` Junio C Hamano
2007-11-11 12:35         ` Andreas Ericsson
2007-11-12  9:03         ` David Symonds
2007-11-12  9:12           ` Johannes Sixt
2007-11-12  9:24             ` David Symonds
2007-11-12  9:50               ` Johannes Sixt
2007-11-12  9:52                 ` David Symonds
2007-11-12 10:07                   ` Jakub Narebski
2007-11-12 10:09                   ` [PATCH] Fix preprocessor logic that determines the availablity of strchrnul() Johannes Sixt

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=473434ED.50002@op5.se \
    --to=ae@op5.se \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=madcoder@debian.org \
    --cc=rene.scharfe@lsrfire.ath.cx \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
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;
as well as URLs for NNTP newsgroup(s).