Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <aOX4urXP7VWilzp7@raf.org>
Date: Wed, 8 Oct 2025 16:38:02 +1100
From: raf <musl@....org>
To: musl@...ts.openwall.com
Subject: Re: [PATCH] fnmatch: fix infinite loop when pattern is
 non-character byte

On Wed, Oct 08, 2025 at 05:04:48AM +0200, Thorsten Glaser <tg@...bsd.de> wrote:

> On Wed, 8 Oct 2025, raf wrote:
> 
> >-		for (p=pat; (c=pat_next(p, -1, &inc, flags))!=END && c!='/'; p+=inc);
> >+		for (p=pat; (c=pat_next(p, -1, &inc, flags))!=END && c!=UNMATCHABLE && c!='/'; p+=inc);
> >+		if (c == UNMATCHABLE)
> >+			return FNM_NOMATCH;
> 
> This would have been a *wonderful* chance to clean up that absolute
> footgun of a loop construct (the semicolon even trailing and with
> no comment):
> 
> 		for (p = pat; (c = pat_next(p, -1, &inc, flags)) != END && c != '/'; p += inc)
> 			if (c == UNMATCHABLE)
> 				return (FNM_NOMATCH);
> 
> Just my impression on casual looking at that diff (from outside),
> //mirabilos

I don't think so. That formatting is used in many
places in this code. It's fine. Changing a single
instance would introduce an inconsistency which I think
would be detrimental. But more importantly, it's
presumably Rich's preferred formatting. It fits more
code per screen. While I don't personally appreciate
the benefit in that myself, I'm not going to argue with
it. It's not my project. Any anyway, people who are
susceptible to footguns usually don't choose to
implement the entire C library. :-)

And the code looks surprisingly pleasant to me.

cheers,
raf

Powered by blists - more mailing lists

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.