|
From: | Kapus, Timotej |
Subject: | Re: [bug-patch] Replace some loops with string.h functions |
Date: | Fri, 26 Oct 2018 16:10:19 +0000 |
Hi,
Thanks for your detailed reply. I think I agree with your point about strspn not being widely used.
I've added the line to bootstrap.conf as you suggested and I'm pasting the patch with just rawmemchr replacements below.
Thanks, Timotej
From a9df6914b14f85d56f09c70084bc4b45f02e9a43 Mon Sep 17 00:00:00 2001
From: Timotej Kapus <address@hidden>
Date: Fri, 26 Oct 2018 17:00:52 +0100
Subject: [PATCH] Replace loops with rawmemchr
* src/inp.c Replace loop with rawmemchr
* src/pch.c Replace loop with rawmemchr
* bootstrap.conf Add rawmemchr
---
bootstrap.conf | 1 +
src/inp.c | 3 +--
src/pch.c | 6 ++----
3 files changed, 4 insertions(+), 6 deletions(-)
diff --git a/bootstrap.conf b/bootstrap.conf
index 68cddd7..01b7e39 100644
--- a/bootstrap.conf
+++ b/bootstrap.conf
@@ -50,6 +50,7 @@ maintainer-makefile
malloc
manywarnings
memchr
+rawmemchr
minmax
mkdirat
nstrftime
diff --git a/src/inp.c b/src/inp.c
index 32d0919..0480e1d 100644
--- a/src/inp.c
+++ b/src/inp.c
@@ -488,8 +488,7 @@ ifetch (lin line, bool whichbuf, size_t *psize)
if (line == input_lines)
*psize = last_line_size;
else {
- for (q = p; *q++ != '\n'; )
- /* do nothing */ ;
+ q = rawmemchr(p,'\n') + 1;
*psize = q - p;
}
return p;
diff --git a/src/pch.c b/src/pch.c
index a500ad9..eca9531 100644
--- a/src/pch.c
+++ b/src/pch.c
@@ -1235,8 +1235,7 @@ another_hunk (enum diff difftype, bool rev)
if (*s == ' ')
{
p_c_function = s;
- while (*s != '\n')
- s++;
+ s = rawmemchr(s,'\n');
*s = '\0';
p_c_function = savestr (p_c_function);
if (! p_c_function)
@@ -1654,8 +1653,7 @@ another_hunk (enum diff difftype, bool rev)
if (*s++ == '@' && *s == ' ')
{
p_c_function = s;
- while (*s != '\n')
- s++;
+ s = rawmemchr(s, '\n');
*s = '\0';
p_c_function = savestr (p_c_function);
if (! p_c_function)
--
2.7.4
Od: Bruno Haible <address@hidden>
Poslano: petek, 26. oktober 2018 16:43:45 Za: address@hidden Kp: Kapus, Timotej; Cadar, Cristian Zadeva: Re: [bug-patch] Replace some loops with string.h functions Hi,
Kapus Timotej wrote: > @@ -488,8 +488,7 @@ ifetch (lin line, bool whichbuf, size_t *psize) > if (line == input_lines) > *psize = last_line_size; > else { > - for (q = p; *q++ != '\n'; ) > - /* do nothing */ ; > + q = rawmemchr(p,'\n') + 1; > *psize = q - p; > } > return p; This looks OK to me. But rawmemchr is not portable; therefore you would also need to import it from gnulib (a one-line addition in bootstrap.conf). > @@ -2348,14 +2343,12 @@ get_ed_command_letter (char const *line) > > if (ISDIGIT (*p)) > { > - while (ISDIGIT (*++p)) > - /* do nothing */ ; > + p += strspn(p + 1, "0123456789") + 1; > if (*p == ',') > { > if (! ISDIGIT (*++p)) > return 0; > - while (ISDIGIT (*++p)) > - /* do nothing */ ; > + p += strspn(p + 1, "0123456789") + 1; > pair = true; > } > } > @@ -2384,8 +2377,7 @@ get_ed_command_letter (char const *line) > return 0; > } > > - while (*p == ' ' || *p == '\t') > - p++; > + p += strspn(p, " \t"); > if (*p == '\n') > return letter; > return 0; I don't think this would improve maintainability. The reason is that these functions strspn, strcspn are not widely used enough, that every maintainer knows them by heart. Take me as an example: I have been coding GNU programs in C for 26 years. I even wrote the manual pages for wcsspn and wcscspn. And yet, I can still not remember what each of these functions actually does and what it returns in case of search failure. Whereas understanding a simple open-coded loop while (*p == ' ' || *p == '\t') p++; is done in 5 seconds, without requiring looking up any man page. And it's more extensible (in case another condition needs to be added in the loop later). Finally, regarding efficiency: As you already noted, strspn(p + 1, "0123456789") will compare each byte against '0', then against '1', then against '2', and so on - without the obvious optimizations that went into the ISDIGIT macro. Bruno |
[Prev in Thread] | Current Thread | [Next in Thread] |