[PATCH] vi: fix regex search compilation error

Bernhard Reutner-Fischer rep.dot.nop at gmail.com
Tue Jul 13 22:51:20 UTC 2021


On Tue, 13 Jul 2021 21:42:05 +0100
Ron Yorston <rmy at pobox.com> wrote:

> Bernhard Reutner-Fischer wrote:
> >If i'm not mistaken, the s++ is fully redundant here although it seems
> >that my gcc-11 does not optimize it like if we manually spell it out
> >like in the attached?  
> 
> I don't think it's possible to avoid two increments.  When a backslash
> is detected we want to move past it and the following character.
> 
> Consider:
> 
>    strchr_backslash("\\/abc/def", '/');
> 
> The original code and Denys' rewrite return a pointer to "/def"; your
> suggestion returns a pointer to "/abc/def".

@@ -2686,10 +2686,9 @@ static char *strchr_backslash(const char *s, int
c) while (*s) {
 		if (*s == c)
 			return (char *)s;
-		if (*s == '\\') # 1
-			if (*++s == '\0') # 2
+		if (*s++ == '\\') # 3
+			if (*s == '\0') # 4
 				break;
-		s++; # 5
 	}
 	return NULL;

How come? Obviously i'm slow today and in addition blind.
So..
1) if *s == backslash
increment s
old: if the incremented s points to 0 (when we break, #2)
new: always (#3)
2) If *s was a backslash
old: if the incremented *s was 0 (#2)
new: if the previously incremented *s was 0 (#3)
then break and return NULL

ah oh in the old code we double increment (only) if the first was a
backslash _not_ followed by a nul.
hmz. So is there a more elegant way to express that which i don't see
right now?

> 
> >Must be marked noinline to avoid inlining it twice it seems?!
> >If that double inlining really happens then the implied code growth
> >even for -Os is a different bug..  
> 
> Hmm, yes, that's rather an odd thing for the compiler to do.

Bad default for -Os for inline limit growth or insns or the like.
Someone should try max-inline-insns-auto i suppose and fix the defaults
to take number of params (passed in regs) or whatnot into account..
But i thought we had max-inline-insns-auto set to 0 or 1 already but
maybe that was an old tree of mine or another, older compiler version..
> 
> Ron



More information about the busybox mailing list