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
next prev 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.