[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Wed, 9 Sep 2020 02:07:47 -0400
From: Rich Felker <dalias@...c.org>
To: musl@...ts.openwall.com
Subject: Re: riscv32 v2
On Mon, Sep 07, 2020 at 09:54:56PM -0400, Rich Felker wrote:
> On Fri, Sep 04, 2020 at 01:48:19AM -0400, Stefan O'Rear wrote:
> > From cd57a6b47783c5302f931e543b608cb3ba58387d Mon Sep 17 00:00:00 2001
> > From: Stefan O'Rear <sorear@...tmail.com>
> > Date: Thu, 3 Sep 2020 03:59:59 -0400
> > Subject: [PATCH 06/14] Only call fstatat if defined
> >
> > riscv32 and future architectures lack it.
> > ---
> > src/stat/fchmodat.c | 23 ++++++++++++++++++++---
> > src/stat/fstatat.c | 6 ++++++
> > src/stdio/tempnam.c | 9 +++++++--
> > src/stdio/tmpnam.c | 9 +++++++--
> > src/time/__map_file.c | 19 +++++++++++++++----
> > 5 files changed, 55 insertions(+), 11 deletions(-)
> >
> > diff --git a/src/stat/fchmodat.c b/src/stat/fchmodat.c
> > index 4ee00b0a..857e84e5 100644
> > --- a/src/stat/fchmodat.c
> > +++ b/src/stat/fchmodat.c
> > @@ -1,8 +1,10 @@
> > #include <sys/stat.h>
> > #include <fcntl.h>
> > #include <errno.h>
> > +#include <stdint.h>
> > #include "syscall.h"
> > #include "kstat.h"
> > +#include "statx.h"
> >
> > int fchmodat(int fd, const char *path, mode_t mode, int flag)
> > {
> > @@ -11,13 +13,22 @@ int fchmodat(int fd, const char *path, mode_t mode, int flag)
> > if (flag != AT_SYMLINK_NOFOLLOW)
> > return __syscall_ret(-EINVAL);
> >
> > - struct kstat st;
> > int ret, fd2;
> > char proc[15+3*sizeof(int)];
> >
> > +#ifdef SYS_fstatat
> > + struct kstat st;
> > if ((ret = __syscall(SYS_fstatat, fd, path, &st, flag)))
> > return __syscall_ret(ret);
> > - if (S_ISLNK(st.st_mode))
> > + mode_t get_mode = st.st_mode;
> > +#else
> > + struct statx st;
> > + if ((ret = __syscall(SYS_statx, fd, path, flag, STATX_TYPE, &st)))
> > + return __syscall_ret(ret);
> > + mode_t get_mode = st.stx_mode;
> > +#endif
> > +
> > + if (S_ISLNK(get_mode))
> > return __syscall_ret(-EOPNOTSUPP);
> >
> > if ((fd2 = __syscall(SYS_openat, fd, path, O_RDONLY|O_PATH|O_NOFOLLOW|O_NOCTTY|O_CLOEXEC)) < 0) {
> > @@ -27,9 +38,15 @@ int fchmodat(int fd, const char *path, mode_t mode, int flag)
> > }
> >
> > __procfdname(proc, fd2);
> > +#ifdef SYS_fstatat
> > ret = __syscall(SYS_fstatat, AT_FDCWD, proc, &st, 0);
> > + get_mode = st.st_mode;
> > +#else
> > + ret = __syscall(SYS_statx, AT_FDCWD, proc, 0, STATX_TYPE, &st);
> > + get_mode = st.stx_mode;
> > +#endif
> > if (!ret) {
> > - if (S_ISLNK(st.st_mode)) ret = -EOPNOTSUPP;
> > + if (S_ISLNK(get_mode)) ret = -EOPNOTSUPP;
> > else ret = __syscall(SYS_fchmodat, AT_FDCWD, proc, mode);
> > }
> >
>
> I was just looking at this file for another reason, and wondered why
> we're not just calling the fstatat function here. There's no namespace
> reason not to. According to the description of commit
> c9ebff4736128186121424364c1c62224b02aee3, use of struct kstat here was
> done "as a low-risk fix" right before release of 1.2.0, and I probably
> had in mind changing it to fstatat later and just never got around to
> it.
>
> The same could be done for tempnam (POSIX) but not tmpnam (C) or
> __map_file (used in plain C interfaces like setlocale and time
> functions) without adding a namespace-safe alias for fstatat.
>
> Of course this does pull in more code that's not needed, so maybe your
> version of the change is best, and maybe this is what I had in mind
> when I hesitated to make the bigger change back in February. I don't
> like that knowledge of different syscall alternatives leaks all over
> the place, but I also don't like increasing code size unnecessarily on
> all archs...
Not something that requires action, but possible streamlining I just
discovered: in places where lstat is being used to probe file
existence (tmpnam, etc.), SYS_readlink also works, yields smaller
code, and is #ifdef-free. You get the same ENOENT for nonexistence,
and success or EINVAL indicating existence as a symlink or
non-symlink, respectively.
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.
