[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20190806020320.GR9017@brightrain.aerifal.cx>
Date: Mon, 5 Aug 2019 22:03:20 -0400
From: Rich Felker <dalias@...c.org>
To: musl@...ts.openwall.com
Subject: Re: [PATCH] In glob(), do not require that the target of a
symlink exists.
On Mon, Jul 22, 2019 at 08:37:38PM -0400, James Y Knight wrote:
> Previously, when given a trailing path component with no
> metacharacters, glob would call stat to check if the provided filename
> existed, which would fail on a broken symlink. When expanding a
> pattern however, glob would trust the list of files returned by
> readdir, and thus would return broken symlinks.
>
> Now, be consistent and allow broken symlinks in both cases, by using
> lstat to determine file existence.
>
> If GLOB_MARK is specified, stat must still be used to determine
> whether a given name refers to a directory or file. But if that fails,
> a symlink is simply considered to be a file for marking purposes.
> From 7ef0fabf173f4d29461b01ccfb05c1f19977ba37 Mon Sep 17 00:00:00 2001
> From: James Y Knight <jyknight@...gle.com>
> Date: Thu, 11 Jul 2019 16:55:17 -0400
> Subject: [PATCH] In glob(), do not require that the target of a symlink
> exists.
>
> Previously, when given a trailing path component with no
> metacharacters, glob would call stat to check if the provided filename
> existed, which would fail on a broken symlink. When expanding a
> pattern however, glob would trust the list of files returned by
> readdir, and thus would return broken symlinks.
>
> Now, be consistent and allow broken symlinks in both cases, by using
> lstat to determine file existence.
>
> If GLOB_MARK is specified, stat must still be used to determine
> whether a given name refers to a directory or file. But if that fails,
> a symlink is simply considered to be a file for marking purposes.
> ---
> src/regex/glob.c | 35 +++++++++++++++++++++++++----------
> 1 file changed, 25 insertions(+), 10 deletions(-)
>
> diff --git a/src/regex/glob.c b/src/regex/glob.c
> index aa1c6a44..01b178dc 100644
> --- a/src/regex/glob.c
> +++ b/src/regex/glob.c
> @@ -88,18 +88,33 @@ static int do_glob(char *buf, size_t pos, int type, char *pat, int flags, int (*
> }
> buf[pos] = 0;
> if (!*pat) {
> - /* If we consumed any components above, or if GLOB_MARK is
> - * requested and we don't yet know if the match is a dir,
> - * we must call stat to confirm the file exists and/or
> - * determine its type. */
> + /* If we're in at the end of the pattern, check if the file
> + * exists. And, if GLOB_MARK is requested, determine if it's a
> + * directory. */
This is one of the comment changes I was confused about; it seems less
informative. The point of the original comment was not describing the
controlling expression just above it, but the conditions on not being
able to use the caller-passed type -- having consumed any new literal
components from the pattern above is what precludes it.
> struct stat st;
> - if ((flags & GLOB_MARK) && type==DT_LNK) type = 0;
> - if (!type && stat(buf, &st)) {
> - if (errno!=ENOENT && (errfunc(buf, errno) || (flags & GLOB_ERR)))
> - return GLOB_ABORTED;
> - return 0;
> + if (flags & GLOB_MARK) {
> + /* We need to know if the name refers to a directory
> + * (after resolving symlinks).
> + *
> + * However, broken symlinks should be considered to be a
> + * file, rather than non-existent or an error, so if
> + * stat fails, we just don't modify type. (And lstat
> + * will be called below if required.)
> + */
> + if ((!type || type==DT_LNK) && stat(buf, &st) == 0) {
!stat
> + if(S_ISDIR(st.st_mode)) type = DT_DIR;
> + else type = DT_REG;
> + }
> + }
> + if (!type) {
> + /* If we're don't already know that the file exists,
> + * confirm its presence. */
> + if (lstat(buf, &st)) {
This could be "!type && lstat(..." to avoid the excessively long lines
below and to better mimic the old structure so that stylistic change
doesn't obscure reading of functional change.
Now that I look at it, the same applies to the above too.
> + if (errno!=ENOENT && (errfunc(buf, errno) || (flags & GLOB_ERR)))
> + return GLOB_ABORTED;
> + return 0;
> + }
> }
> - if (!type && S_ISDIR(st.st_mode)) type = DT_DIR;
> if (append(tail, buf, pos, (flags & GLOB_MARK) && type==DT_DIR))
> return GLOB_NOSPACE;
> return 0;
> --
> 2.22.0.657.g960e92d24f-goog
>
Otherwise I think this looks ok.
Rich
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.
