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
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/
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/