Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Date: Fri, 27 Aug 2021 20:15:46 +0200
From: Ismael Luceno <ismael@...ev.co.uk>
To: musl@...ts.openwall.com
Cc: Rich Felker <dalias@...c.org>
Subject: Re: [PATCH] glob: introduce context struct for do_glob

ok, nevermind the last email, I think it's fine, I was worrying about
the ctx struct being constant but that seems fine...

On 27/Aug/2021 20:13, Ismael Luceno wrote:
> ignore this patch, it's garbage, I'll fix it
> 
> On 27/Aug/2021 20:08, Ismael Luceno wrote:
> > this reduces the function frame by sharing more state in the recursion,
> > and produces a slightly smaller object file with GCC 10.3 on x86_64:
> > 
> >    text    data     bss     dec     hex filename
> >    2303       0       0    2303     8ff glob-ctx.lo
> >    2356       0       0    2356     934 glob-noctx.lo
> > 
> > Signed-off-by: Ismael Luceno <ismael@...ev.co.uk>
> > ---
> >  src/regex/glob.c | 42 +++++++++++++++++++++++++++---------------
> >  1 file changed, 27 insertions(+), 15 deletions(-)
> > 
> > diff --git a/src/regex/glob.c b/src/regex/glob.c
> > index 9de080ed9ccd..85e0f027b21a 100644
> > --- a/src/regex/glob.c
> > +++ b/src/regex/glob.c
> > @@ -32,10 +32,16 @@ static int append(struct match **tail, const char *name, size_t len, int mark)
> >  	return 0;
> >  }
> >  
> > -static int do_glob(char *buf, size_t pos, int type, char *pat, int flags, int (*errfunc)(const char *path, int err), struct match **tail)
> > +struct glob_ctx {
> > +	struct match **tail;
> > +	int flags;
> > +	int (*errfunc)(const char *path, int err);
> > +};
> > +
> > +static int do_glob(char *buf, size_t pos, int type, char *pat, const struct glob_ctx *restrict ctx)
> >  {
> >  	/* If GLOB_MARK is unused, we don't care about type. */
> > -	if (!type && !(flags & GLOB_MARK)) type = DT_REG;
> > +	if (!type && !(ctx->flags & GLOB_MARK)) type = DT_REG;
> >  
> >  	/* Special-case the remaining pattern being all slashes, in
> >  	 * which case we can use caller-passed type if it's a dir. */
> > @@ -55,7 +61,7 @@ static int do_glob(char *buf, size_t pos, int type, char *pat, int flags, int (*
> >  			break;
> >  		} else if (pat[i] == '[') {
> >  			in_bracket = 1;
> > -		} else if (pat[i] == '\\' && !(flags & GLOB_NOESCAPE)) {
> > +		} else if (pat[i] == '\\' && !(ctx->flags & GLOB_NOESCAPE)) {
> >  			/* Backslashes inside a bracket are (at least by
> >  			 * our interpretation) non-special, so if next
> >  			 * char is ']' we have a complete expression. */
> > @@ -100,23 +106,23 @@ static int do_glob(char *buf, size_t pos, int type, char *pat, int flags, int (*
> >  		 * or if that fails, use lstat for determining existence to
> >  		 * avoid false negatives in the case of broken symlinks. */
> >  		struct stat st;
> > -		if ((flags & GLOB_MARK) && (!type||type==DT_LNK) && !stat(buf, &st)) {
> > +		if ((ctx->flags & GLOB_MARK) && (!type||type==DT_LNK) && !stat(buf, &st)) {
> >  			if (S_ISDIR(st.st_mode)) type = DT_DIR;
> >  			else type = DT_REG;
> >  		}
> >  		if (!type && lstat(buf, &st)) {
> > -			if (errno!=ENOENT && (errfunc(buf, errno) || (flags & GLOB_ERR)))
> > +			if (errno!=ENOENT && (ctx->errfunc(buf, errno) || (ctx->flags & GLOB_ERR)))
> >  				return GLOB_ABORTED;
> >  			return 0;
> >  		}
> > -		if (append(tail, buf, pos, (flags & GLOB_MARK) && type==DT_DIR))
> > +		if (append(ctx->tail, buf, pos, (ctx->flags & GLOB_MARK) && type==DT_DIR))
> >  			return GLOB_NOSPACE;
> >  		return 0;
> >  	}
> >  	char *p2 = strchr(pat, '/'), saved_sep = '/';
> >  	/* Check if the '/' was escaped and, if so, remove the escape char
> >  	 * so that it will not be unpaired when passed to fnmatch. */
> > -	if (p2 && !(flags & GLOB_NOESCAPE)) {
> > +	if (p2 && !(ctx->flags & GLOB_NOESCAPE)) {
> >  		char *p;
> >  		for (p=p2; p>pat && p[-1]=='\\'; p--);
> >  		if ((p2-p)%2) {
> > @@ -126,7 +132,7 @@ static int do_glob(char *buf, size_t pos, int type, char *pat, int flags, int (*
> >  	}
> >  	DIR *dir = opendir(pos ? buf : ".");
> >  	if (!dir) {
> > -		if (errfunc(buf, errno) || (flags & GLOB_ERR))
> > +		if (ctx->errfunc(buf, errno) || (ctx->flags & GLOB_ERR))
> >  			return GLOB_ABORTED;
> >  		return 0;
> >  	}
> > @@ -142,22 +148,22 @@ static int do_glob(char *buf, size_t pos, int type, char *pat, int flags, int (*
> >  
> >  		if (p2) *p2 = 0;
> >  
> > -		int fnm_flags= ((flags & GLOB_NOESCAPE) ? FNM_NOESCAPE : 0)
> > -			| ((!(flags & GLOB_PERIOD)) ? FNM_PERIOD : 0);
> > +		int fnm_flags= ((ctx->flags & GLOB_NOESCAPE) ? FNM_NOESCAPE : 0)
> > +			| ((!(ctx->flags & GLOB_PERIOD)) ? FNM_PERIOD : 0);
> >  
> >  		if (fnmatch(pat, de->d_name, fnm_flags))
> >  			continue;
> >  
> >  		/* With GLOB_PERIOD, don't allow matching . or .. unless
> >  		 * fnmatch would match them with FNM_PERIOD rules in effect. */
> > -		if (p2 && (flags & GLOB_PERIOD) && de->d_name[0]=='.'
> > +		if (p2 && (ctx->flags & GLOB_PERIOD) && de->d_name[0]=='.'
> >  		    && (!de->d_name[1] || de->d_name[1]=='.' && !de->d_name[2])
> >  		    && fnmatch(pat, de->d_name, fnm_flags | FNM_PERIOD))
> >  			continue;
> >  
> >  		memcpy(buf+pos, de->d_name, l+1);
> >  		if (p2) *p2 = saved_sep;
> > -		int r = do_glob(buf, pos+l, de->d_type, p2 ? p2 : "", flags, errfunc, tail);
> > +		int r = do_glob(buf, pos+l, de->d_type, p2 ? p2 : "", ctx);
> >  		if (r) {
> >  			closedir(dir);
> >  			return r;
> > @@ -166,7 +172,7 @@ static int do_glob(char *buf, size_t pos, int type, char *pat, int flags, int (*
> >  	int readerr = errno;
> >  	if (p2) *p2 = saved_sep;
> >  	closedir(dir);
> > -	if (readerr && (errfunc(buf, errno) || (flags & GLOB_ERR)))
> > +	if (readerr && (ctx->errfunc(buf, errno) || (ctx->flags & GLOB_ERR)))
> >  		return GLOB_ABORTED;
> >  	errno = old_errno;
> >  	return 0;
> > @@ -248,8 +254,14 @@ int glob(const char *restrict pat, int flags, int (*errfunc)(const char *path, i
> >  		char *s = p;
> >  		if ((flags & (GLOB_TILDE | GLOB_TILDE_CHECK)) && *p == '~')
> >  			error = expand_tilde(&s, buf, &pos);
> > -		if (!error)
> > -			error = do_glob(buf, pos, 0, s, flags, errfunc, &tail);
> > +		if (!error) {
> > +			struct glob_ctx ctx = {
> > +				.tail = &tail,
> > +				.flags = flags,
> > +				.errfunc = errfunc,
> > +			};
> > +			error = do_glob(buf, pos, 0, s, &ctx);
> > +		}
> >  		free(p);
> >  	}
> >  
> > -- 
> > 2.33.0
> > 
> 
> -- 
> Ismael Luceno
> http://iodev.co.uk/

-- 
Ismael Luceno
http://iodev.co.uk/

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.