[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20190723200708.GA25787@brightrain.aerifal.cx>
Date: Tue, 23 Jul 2019 16:07:08 -0400
From: Rich Felker <dalias@...c.org>
To: musl@...ts.openwall.com
Subject: Re: [PATCH] glob: implement GLOB_TILDE
On Tue, Jul 23, 2019 at 08:33:54PM +0200, Ismael Luceno wrote:
> Signed-off-by: Ismael Luceno <ismael@...ev.co.uk>
> ---
> include/glob.h | 1 +
> src/regex/glob.c | 26 ++++++++++++++++++++++++++
> 2 files changed, 27 insertions(+)
>
> diff --git a/include/glob.h b/include/glob.h
> index 76f6c1c68a23..fc8106b20c5b 100644
> --- a/include/glob.h
> +++ b/include/glob.h
> @@ -30,6 +30,7 @@ void globfree(glob_t *);
> #define GLOB_APPEND 0x20
> #define GLOB_NOESCAPE 0x40
> #define GLOB_PERIOD 0x80
> +#define GLOB_TILDE 0x100
>
> #define GLOB_NOSPACE 1
> #define GLOB_ABORTED 2
> diff --git a/src/regex/glob.c b/src/regex/glob.c
> index aa1c6a4482ee..77b81f707420 100644
> --- a/src/regex/glob.c
> +++ b/src/regex/glob.c
> @@ -8,6 +8,8 @@
> #include <stdlib.h>
> #include <errno.h>
> #include <stddef.h>
> +#include <unistd.h>
> +#include "../passwd/pwf.h"
>
> struct match
> {
> @@ -35,6 +37,30 @@ static int do_glob(char *buf, size_t pos, int type, char *pat, int flags, int (*
> /* If GLOB_MARK is unused, we don't care about type. */
> if (!type && !(flags & GLOB_MARK)) type = DT_REG;
>
> + if ((flags & GLOB_TILDE) && *pat == '~') {
> + struct passwd pw, *res;
> + char *buf = NULL;
> + size_t len = 0;
> + char *name_end = strchr(++pat, '/');
> + if (name_end)
> + *name_end = 0;
> + uid_t uid = *pat ? 0 : getuid();
> + int err = __getpw_a(pat, uid, &pw, &buf, &len, &res);
> + if (!err && res) {
> + while (pos < PATH_MAX - 1 && *pw.pw_dir)
> + buf[pos++] = *pw.pw_dir++;
> + }
> + free(buf);
> + if (err)
> + return GLOB_NOSPACE;
> + if (!res)
> + return 0;
> + if (name_end) {
> + pat = name_end;
> + *pat = '/';
> + }
> + }
This should almost surely go in the top-level glob function rather
than the recusive function, where the original strdup of pat happens.
As you've written it, I think it wrongly applies tilde expansion to
each path component into which recursion happens, but not to ones
where it doesn't, whereas it should only apply to the first component.
Doing this in the recursive function also has the problem of exploding
stack usage.
One thing I don't see is how the code is working at all right now,
because you've shadowed the argument buf with a locally scoped
variable by the same name that's the working space buffer passed to
__getpw_a. The result directory is never copied out into the actual
buf array. One thing to note at this point: it's generally preferable
to use the public interfaces (getpwnam_r) rather than the internal
ones from a different sybsystem of libc (__getpw_a) unless there's a
really compelling reason not to. Here I think it would actually be
easier using the public interface -- you could reuse buf for the
buffer space you pass to getpwnam_r, then memmove from pw_dir (which
lies somewhere inside buf, you don't know where) to the start of buf.
There's also a matter of intended behavor: my understanding is that
plain ~ without a name is supposed to expand to $HOME, not lookup the
caller's passwd entry based on uid (which is broken on setups where
you have multiple login accounts with the same uid). The man page
doesn't document this at all. I looked just now in the glibc manual
and it documents the behavior in terms of getlogin+getpwnam, which is
utterly broken -- it doesn't work at all in the absence of a login
session associated with a terminal (except in musl where we wrongly
just return getenv("LOGNAME"), a known/open bug). and it's also
inconsistent with the standard shell behavior, which is to use $HOME:
http://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_06_01
So I feel like, if we implement GLOB_TILDE, we should do it the way
that's consistent with the standard meaning of ~, using $HOME, not the
glibc implementation. This is also much more user-friendly, in that it
honors manual override of $HOME.
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.
