From: Rich Felker <dalias@libc.org>
To: musl@lists.openwall.com
Subject: Re: [musl] riscv32 v2
Date: Mon, 7 Sep 2020 21:54:56 -0400 [thread overview]
Message-ID: <20200908015456.GT3265@brightrain.aerifal.cx> (raw)
In-Reply-To: <e792658a-fbaa-48bd-8704-7f9d2ec5a4ef@www.fastmail.com>
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@fastmail.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...
Rich
next prev parent reply other threads:[~2020-09-08 1:55 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-09-04 5:48 Stefan O'Rear
2020-09-07 10:47 ` Stefan O'Rear
2020-09-07 18:06 ` Rich Felker
2020-09-07 21:35 ` Arnd Bergmann
2020-09-07 21:45 ` Rich Felker
2020-09-07 21:58 ` Arnd Bergmann
2020-09-07 22:11 ` Rich Felker
2020-09-07 22:30 ` Arnd Bergmann
2020-09-08 1:02 ` Rich Felker
2020-09-08 7:00 ` Arnd Bergmann
2020-09-07 11:27 ` Stefan O'Rear
2020-09-07 18:09 ` Rich Felker
2020-09-08 1:54 ` Rich Felker [this message]
2020-09-09 6:07 ` Rich Felker
2020-09-09 20:28 ` Rich Felker
2020-09-09 21:28 ` Palmer Dabbelt
2020-09-09 21:36 ` Rich Felker
2020-09-09 23:08 ` Palmer Dabbelt
2020-09-10 7:36 ` Arnd Bergmann
2020-09-10 10:01 ` Vincenzo Frascino
2020-09-11 0:08 ` Palmer Dabbelt
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200908015456.GT3265@brightrain.aerifal.cx \
--to=dalias@libc.org \
--cc=musl@lists.openwall.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).