On 27/03/2018 18:04, Herbert Xu wrote:
On Tue, Mar 27, 2018 at 05:54:53PM +0200, Harald van Dijk wrote:


I was thinking about not making backslashes set metaflag in expmeta(): when
the pathname component doesn't include *, ?, or [, but does include
backslashes, then the if (metaflag == 0) block could handle that as long as
it performs the lstat64() check unconditionally. There's no need to go
through the opendir()/readdir()/closedir() path for that case. Since
expmeta() is bypassed for words not containing any potentially-magic
characters, the impact might be small enough.

Honestly such backslashes should be rare enough that I wouldn't
bother with such an optimisation.

Backslashes coming from parameters, sure, but backslashes introduced by preglob(), I'm not so sure.

Regardless of whether metaflag is set, it would mean things like 'set "["'
would start hitting the FS unnecessarily, if I understand correctly: the
preglob()-expanded pattern is '\[', and expmeta() can no longer tell this
apart from $v where v='\[' where a FS check would be required.

No it shouldn't.  We only mark [ is meta if there is a matching ].

The [ character would mark the whole string possibly-meta in expandmeta(), and the CTLESC wouldn't be seen there. Then, preglob() turns it into \[, and expmeta() wouldn't take [ as a meta character, but would take \ as a meta character.

Perhaps preglob() should just be avoided, and expmeta() taught to respect
both '\\' and CTLESC. '\\' would be a metacharacter and require a FS hit,
CTLESC wouldn't require it. This would also avoid some memory allocations.

We need preglob for glob(3).  I want to minimise the amount of code
difference between the glob path and the expmeta path.

Then, how about moving some of expmeta()'s checks to return early outside of it, so that they can also be done in the glob() case?

     if (!strpbrk(str->text, metachars))
         goto nometa;

in expandmeta() is done before preglob() is called. Here, it is still not
necessary to include CTLESC.

We could move it after preglob.

Assuming backslash is added to metachars, which was needed anyway:

Regardless of whether the check is done before or after preglob(), it would never skip expmeta() when it shouldn't. If it's kept before preglob(), then it will correctly skip expmeta() in more cases than if it's moved after it, so I don't think there's any benefit in moving it.

Cheers,
Harald van Dijk
--
To unsubscribe from this list: send the line "unsubscribe dash" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to