[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [bug-patch] Replace some loops with string.h functions
From: |
Bruno Haible |
Subject: |
Re: [bug-patch] Replace some loops with string.h functions |
Date: |
Fri, 26 Oct 2018 17:43:45 +0200 |
User-agent: |
KMail/5.1.3 (Linux/4.4.0-137-generic; KDE/5.18.0; x86_64; ; ) |
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