bug-patch
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [bug-patch] Replace some loops with string.h functions


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


reply via email to

[Prev in Thread] Current Thread [Next in Thread]