* [PATCH v4 1/2] add internal aliases __opendir, __readdir and __closedir @ 2019-09-06 19:40 Ismael Luceno 2019-09-06 19:40 ` [PATCH v4 2/2] glob: implement GLOB_ALTDIRFUNC et al Ismael Luceno 0 siblings, 1 reply; 3+ messages in thread From: Ismael Luceno @ 2019-09-06 19:40 UTC (permalink / raw) To: musl; +Cc: Ismael Luceno Signed-off-by: Ismael Luceno <ismael@iodev.co.uk> --- src/dirent/closedir.c | 4 +++- src/dirent/opendir.c | 4 +++- src/dirent/readdir.c | 5 +++-- src/include/dirent.h | 10 ++++++++++ 4 files changed, 19 insertions(+), 4 deletions(-) create mode 100644 src/include/dirent.h diff --git a/src/dirent/closedir.c b/src/dirent/closedir.c index e794ae9ca44b..f4249f56e210 100644 --- a/src/dirent/closedir.c +++ b/src/dirent/closedir.c @@ -3,9 +3,11 @@ #include <stdlib.h> #include "__dirent.h" -int closedir(DIR *dir) +int __closedir(DIR *dir) { int ret = close(dir->fd); free(dir); return ret; } + +weak_alias(__closedir, closedir); diff --git a/src/dirent/opendir.c b/src/dirent/opendir.c index 5cb84e303fee..4123c81994cd 100644 --- a/src/dirent/opendir.c +++ b/src/dirent/opendir.c @@ -5,7 +5,7 @@ #include "__dirent.h" #include "syscall.h" -DIR *opendir(const char *name) +DIR *__opendir(const char *name) { int fd; DIR *dir; @@ -19,3 +19,5 @@ DIR *opendir(const char *name) dir->fd = fd; return dir; } + +weak_alias(__opendir, opendir); diff --git a/src/dirent/readdir.c b/src/dirent/readdir.c index 569fc7057737..cb34a258569c 100644 --- a/src/dirent/readdir.c +++ b/src/dirent/readdir.c @@ -7,7 +7,7 @@ typedef char dirstream_buf_alignment_check[1-2*(int)( offsetof(struct __dirstream, buf) % sizeof(off_t))]; -struct dirent *readdir(DIR *dir) +struct dirent *__readdir(DIR *dir) { struct dirent *de; @@ -26,4 +26,5 @@ struct dirent *readdir(DIR *dir) return de; } -weak_alias(readdir, readdir64); +weak_alias(__readdir, readdir64); +weak_alias(__readdir, readdir); diff --git a/src/include/dirent.h b/src/include/dirent.h new file mode 100644 index 000000000000..918e123566d4 --- /dev/null +++ b/src/include/dirent.h @@ -0,0 +1,10 @@ +#ifndef DIRENT_H +#define DIRENT_H + +#include "../../include/dirent.h" + +hidden int __closedir(DIR *); +hidden DIR *__opendir(const char *); +hidden struct dirent *__readdir(DIR *); + +#endif -- 2.23.0 ^ permalink raw reply [flat|nested] 3+ messages in thread
* [PATCH v4 2/2] glob: implement GLOB_ALTDIRFUNC et al 2019-09-06 19:40 [PATCH v4 1/2] add internal aliases __opendir, __readdir and __closedir Ismael Luceno @ 2019-09-06 19:40 ` Ismael Luceno 2019-09-06 21:42 ` Rich Felker 0 siblings, 1 reply; 3+ messages in thread From: Ismael Luceno @ 2019-09-06 19:40 UTC (permalink / raw) To: musl; +Cc: Ismael Luceno Signed-off-by: Ismael Luceno <ismael@iodev.co.uk> --- Notes: Changes since v3: - Wrap pointers used by GLOB_ALTDIRFUNC on the header to protect from namespace contamination. GNU and BSD code shouldn't be using "dirent", so it should be safe. Changes since v2: - Rebased Changes since v1: - Avoid overwriting the function pointers in glob_t - Wrap {open,read}dir too include/glob.h | 10 ++++++++++ src/regex/glob.c | 47 +++++++++++++++++++++++++++++++++++++---------- 2 files changed, 47 insertions(+), 10 deletions(-) diff --git a/include/glob.h b/include/glob.h index 0ff70bdfeef2..26ea65cd1097 100644 --- a/include/glob.h +++ b/include/glob.h @@ -16,7 +16,16 @@ typedef struct { char **gl_pathv; size_t gl_offs; int __dummy1; + +#if defined(_BSD_SOURCE) || defined(_GNU_SOURCE) + void (*gl_closedir)(void *); + struct dirent *(*gl_readdir)(void *); + void *(*gl_opendir)(const char *); + int (*gl_lstat)(const char *__restrict, struct stat *__restrict); + int (*gl_stat)(const char *__restrict, struct stat *__restrict); +#else void *__dummy2[5]; +#endif } glob_t; int glob(const char *__restrict, int, int (*)(const char *, int), glob_t *__restrict); @@ -31,6 +40,7 @@ void globfree(glob_t *); #define GLOB_NOESCAPE 0x40 #define GLOB_PERIOD 0x80 +#define GLOB_ALTDIRFUNC 0x0200 #define GLOB_NOMAGIC 0x0800 #define GLOB_TILDE 0x1000 #define GLOB_TILDE_CHECK 0x4000 diff --git a/src/regex/glob.c b/src/regex/glob.c index 7780e21ee113..4f329f053fe0 100644 --- a/src/regex/glob.c +++ b/src/regex/glob.c @@ -1,7 +1,7 @@ #define _BSD_SOURCE +#include <sys/stat.h> #include <glob.h> #include <fnmatch.h> -#include <sys/stat.h> #include <dirent.h> #include <limits.h> #include <string.h> @@ -11,6 +11,14 @@ #include <unistd.h> #include <pwd.h> +struct dirfn { + void (*closedir)(void *); + struct dirent *(*readdir)(void *); + void *(*opendir)(const char *); + int (*lstat)(const char *restrict, struct stat *restrict); + int (*stat)(const char *restrict, struct stat *restrict); +}; + struct match { struct match *next; @@ -32,7 +40,7 @@ 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) +static int do_glob(char *buf, size_t pos, int type, char *pat, int flags, int (*errfunc)(const char *path, int err), const struct dirfn * const restrict dirfn, struct match **tail) { /* If GLOB_MARK is unused, we don't care about type. */ if (!type && !(flags & GLOB_MARK)) type = DT_REG; @@ -100,11 +108,11 @@ 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 ((flags & GLOB_MARK) && (!type||type==DT_LNK) && !dirfn->stat(buf, &st)) { if (S_ISDIR(st.st_mode)) type = DT_DIR; else type = DT_REG; } - if (!type && lstat(buf, &st)) { + if (!type && dirfn->lstat(buf, &st)) { if (errno!=ENOENT && (errfunc(buf, errno) || (flags & GLOB_ERR))) return GLOB_ABORTED; return 0; @@ -124,7 +132,7 @@ static int do_glob(char *buf, size_t pos, int type, char *pat, int flags, int (* saved_sep = '\\'; } } - DIR *dir = opendir(pos ? buf : "."); + DIR *dir = dirfn->opendir(pos ? buf : "."); if (!dir) { if (errfunc(buf, errno) || (flags & GLOB_ERR)) return GLOB_ABORTED; @@ -132,7 +140,7 @@ static int do_glob(char *buf, size_t pos, int type, char *pat, int flags, int (* } int old_errno = errno; struct dirent *de; - while (errno=0, de=readdir(dir)) { + while (errno=0, de=dirfn->readdir(dir)) { /* Quickly skip non-directories when there's pattern left. */ if (p2 && de->d_type && de->d_type!=DT_DIR && de->d_type!=DT_LNK) continue; @@ -157,15 +165,15 @@ static int do_glob(char *buf, size_t pos, int type, char *pat, int flags, int (* 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 : "", flags, errfunc, dirfn, tail); if (r) { - closedir(dir); + dirfn->closedir(dir); return r; } } int readerr = errno; if (p2) *p2 = saved_sep; - closedir(dir); + dirfn->closedir(dir); if (readerr && (errfunc(buf, errno) || (flags & GLOB_ERR))) return GLOB_ABORTED; errno = old_errno; @@ -224,6 +232,10 @@ static int expand_tilde(char **pat, char *buf, size_t *pos) return 0; } +static void wrap_closedir(void *p) { __closedir(p); } +static struct dirent *wrap_readdir(void *d) { return __readdir(d); } +static void *wrap_opendir(const char *path) { return __opendir(path); } + int glob(const char *restrict pat, int flags, int (*errfunc)(const char *path, int err), glob_t *restrict g) { struct match head = { .next = NULL }, *tail = &head; @@ -231,9 +243,24 @@ int glob(const char *restrict pat, int flags, int (*errfunc)(const char *path, i size_t offs = (flags & GLOB_DOOFFS) ? g->gl_offs : 0; int error = 0; char buf[PATH_MAX]; + struct dirfn dirfn; if (!errfunc) errfunc = ignore_err; + if (flags & GLOB_ALTDIRFUNC) { + dirfn.closedir = g->gl_closedir; + dirfn.readdir = g->gl_readdir; + dirfn.opendir = g->gl_opendir; + dirfn.lstat = g->gl_lstat; + dirfn.stat = g->gl_stat; + } else { + dirfn.closedir = wrap_closedir; + dirfn.readdir = wrap_readdir; + dirfn.opendir = wrap_opendir; + dirfn.lstat = lstat; + dirfn.stat = stat; + } + if (!(flags & GLOB_APPEND)) { g->gl_offs = offs; g->gl_pathc = 0; @@ -249,7 +276,7 @@ int glob(const char *restrict pat, int flags, int (*errfunc)(const char *path, i 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); + error = do_glob(buf, pos, 0, s, flags, errfunc, &dirfn, &tail); free(p); } -- 2.23.0 ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v4 2/2] glob: implement GLOB_ALTDIRFUNC et al 2019-09-06 19:40 ` [PATCH v4 2/2] glob: implement GLOB_ALTDIRFUNC et al Ismael Luceno @ 2019-09-06 21:42 ` Rich Felker 0 siblings, 0 replies; 3+ messages in thread From: Rich Felker @ 2019-09-06 21:42 UTC (permalink / raw) To: musl On Fri, Sep 06, 2019 at 09:40:22PM +0200, Ismael Luceno wrote: > diff --git a/include/glob.h b/include/glob.h > index 0ff70bdfeef2..26ea65cd1097 100644 > --- a/include/glob.h > +++ b/include/glob.h > @@ -16,7 +16,16 @@ typedef struct { > char **gl_pathv; > size_t gl_offs; > int __dummy1; > + > +#if defined(_BSD_SOURCE) || defined(_GNU_SOURCE) > + void (*gl_closedir)(void *); > + struct dirent *(*gl_readdir)(void *); > + void *(*gl_opendir)(const char *); > + int (*gl_lstat)(const char *__restrict, struct stat *__restrict); > + int (*gl_stat)(const char *__restrict, struct stat *__restrict); > +#else > void *__dummy2[5]; > +#endif > } glob_t; I think you need to forward-declare at least struct stat, and possibly also struct dirent, so that they refer to the same types as the declarations elsewhere. At least the argument list is a different scope and declares a *different* struct stat shadowing the file-scope one if the file-scope one is not already declared. With the above change and: > diff --git a/src/regex/glob.c b/src/regex/glob.c > index 7780e21ee113..4f329f053fe0 100644 > --- a/src/regex/glob.c > +++ b/src/regex/glob.c > @@ -1,7 +1,7 @@ > #define _BSD_SOURCE ^^^^^^^^^^^ combined, every use of g->[something] in glob() is now undefined behavior if the caller was using a standard profile where the __dummy2-containing version of glob_t above is the one that's used. That's because it's accessing one struct type via an lvalue of a different struct type. > +#include <sys/stat.h> > #include <glob.h> > #include <fnmatch.h> > -#include <sys/stat.h> Unrelated: this change is gratuitous. > @@ -224,6 +232,10 @@ static int expand_tilde(char **pat, char *buf, size_t *pos) > return 0; > } > > +static void wrap_closedir(void *p) { __closedir(p); } > +static struct dirent *wrap_readdir(void *d) { return __readdir(d); } > +static void *wrap_opendir(const char *path) { return __opendir(path); } > + > int glob(const char *restrict pat, int flags, int (*errfunc)(const char *path, int err), glob_t *restrict g) > { > struct match head = { .next = NULL }, *tail = &head; > @@ -231,9 +243,24 @@ int glob(const char *restrict pat, int flags, int (*errfunc)(const char *path, i > size_t offs = (flags & GLOB_DOOFFS) ? g->gl_offs : 0; ^^^^^^^^^^ These are the accesses that become UB. To make them well-defined, they'd need to be something like G(g,gl_offs), where #define G(g,m) (*(1?(void*)((char*)(g)+offsetof(glob_t,m)):&((glob_t){0}).m) is the cleanest abstraction I can think of for the monstrosity needed to access "something layout-compatible with glob_t, but not glob_t, at address g". Imposing this ugliness to avoid UB is part of why I'm hesitant about this functionality. I think there should be good motivation for it if we want to go forward with it. > int error = 0; > char buf[PATH_MAX]; > + struct dirfn dirfn; > > if (!errfunc) errfunc = ignore_err; > > + if (flags & GLOB_ALTDIRFUNC) { > + dirfn.closedir = g->gl_closedir; > + dirfn.readdir = g->gl_readdir; > + dirfn.opendir = g->gl_opendir; > + dirfn.lstat = g->gl_lstat; > + dirfn.stat = g->gl_stat; > + } else { > + dirfn.closedir = wrap_closedir; > + dirfn.readdir = wrap_readdir; > + dirfn.opendir = wrap_opendir; > + dirfn.lstat = lstat; > + dirfn.stat = stat; > + } > + > if (!(flags & GLOB_APPEND)) { > g->gl_offs = offs; > g->gl_pathc = 0; > @@ -249,7 +276,7 @@ int glob(const char *restrict pat, int flags, int (*errfunc)(const char *path, i > 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); > + error = do_glob(buf, pos, 0, s, flags, errfunc, &dirfn, &tail); > free(p); > } Not necessary for correctness, but if we're adding a context structure that gets passed down to each level of do_glob, we might as well put the errfunc in it too, and possibly also flags and &tail in it, so that we reduce the amount of state at each recursion frame rather than increasing it. It's likely this would make more sense to do as a separate patch though, to keep the diffs clear. Rich ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2019-09-06 21:42 UTC | newest] Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-09-06 19:40 [PATCH v4 1/2] add internal aliases __opendir, __readdir and __closedir Ismael Luceno 2019-09-06 19:40 ` [PATCH v4 2/2] glob: implement GLOB_ALTDIRFUNC et al Ismael Luceno 2019-09-06 21:42 ` Rich Felker
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).