mailing list of musl libc
 help / color / mirror / code / Atom feed
* [musl] [PATCH] glob: introduce context struct for do_glob
@ 2021-08-27 18:08 Ismael Luceno
  2021-08-27 18:13 ` Ismael Luceno
  0 siblings, 1 reply; 3+ messages in thread
From: Ismael Luceno @ 2021-08-27 18:08 UTC (permalink / raw)
  To: musl; +Cc: Rich Felker, Ismael Luceno

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@iodev.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


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [musl] [PATCH] glob: introduce context struct for do_glob
  2021-08-27 18:08 [musl] [PATCH] glob: introduce context struct for do_glob Ismael Luceno
@ 2021-08-27 18:13 ` Ismael Luceno
  2021-08-27 18:15   ` Ismael Luceno
  0 siblings, 1 reply; 3+ messages in thread
From: Ismael Luceno @ 2021-08-27 18:13 UTC (permalink / raw)
  To: musl; +Cc: Rich Felker

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@iodev.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/

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [musl] [PATCH] glob: introduce context struct for do_glob
  2021-08-27 18:13 ` Ismael Luceno
@ 2021-08-27 18:15   ` Ismael Luceno
  0 siblings, 0 replies; 3+ messages in thread
From: Ismael Luceno @ 2021-08-27 18:15 UTC (permalink / raw)
  To: musl; +Cc: Rich Felker

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@iodev.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/

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2021-08-27 18:16 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-27 18:08 [musl] [PATCH] glob: introduce context struct for do_glob Ismael Luceno
2021-08-27 18:13 ` Ismael Luceno
2021-08-27 18:15   ` Ismael Luceno

Code repositories for project(s) associated with this public inbox

	https://git.vuxu.org/mirror/musl/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).