From: Rich Felker <dalias@libc.org>
To: musl@lists.openwall.com
Subject: Re: [musl] [PATCH 06/14] Only call fstatat if defined
Date: Thu, 3 Sep 2020 12:05:02 -0400 [thread overview]
Message-ID: <20200903160502.GD3265@brightrain.aerifal.cx> (raw)
In-Reply-To: <20200903112309.102601-7-sorear@fastmail.com>
On Thu, Sep 03, 2020 at 07:23:01AM -0400, Stefan O'Rear wrote:
> riscv32 and future architectures lack it.
> ---
> src/stat/fchmodat.c | 22 +++++++++++++++++++++-
> src/stat/fstatat.c | 6 ++++++
> src/stdio/tempnam.c | 7 +++++++
> src/stdio/tmpnam.c | 7 +++++++
> src/time/__map_file.c | 13 ++++++++++++-
> 5 files changed, 53 insertions(+), 2 deletions(-)
>
> diff --git a/src/stat/fchmodat.c b/src/stat/fchmodat.c
> index 4ee00b0a..bcd44cc8 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,14 +13,24 @@ 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))
> return __syscall_ret(-EOPNOTSUPP);
> +#else
> + struct statx st;
> + if ((ret = __syscall(SYS_statx, fd, path, flag, STATX_TYPE, &st)))
> + return __syscall_ret(ret);
> +
> + if (S_ISLNK(st.stx_mode))
> + return __syscall_ret(-EOPNOTSUPP);
> +#endif
Can you do this as mode_t mode = st.st[x]_mode; inside the conditional
block and unconditional if (S_ISLNK(mode)) ... outside? This ensures
that the actual logic is not duplicated so it can't become
inconsistent.
> @@ -27,11 +39,19 @@ 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);
> if (!ret) {
> if (S_ISLNK(st.st_mode)) ret = -EOPNOTSUPP;
> else ret = __syscall(SYS_fchmodat, AT_FDCWD, proc, mode);
> }
> +#else
> + ret = __syscall(SYS_statx, AT_FDCWD, proc, 0, STATX_TYPE, &st);
> + if (!ret) {
> + if (S_ISLNK(st.stx_mode)) ret = -EOPNOTSUPP;
> + else ret = __syscall(SYS_fchmodat, AT_FDCWD, proc, mode);
> + }
> +#endif
Same here where even more logic is duplicated.
> diff --git a/src/stat/fstatat.c b/src/stat/fstatat.c
> index 230a83fc..0486f21a 100644
> --- a/src/stat/fstatat.c
> +++ b/src/stat/fstatat.c
> @@ -45,6 +45,7 @@ static int fstatat_statx(int fd, const char *restrict path, struct stat *restric
> return 0;
> }
>
> +#ifdef SYS_fstatat
> static int fstatat_kstat(int fd, const char *restrict path, struct stat *restrict st, int flag)
> {
> int ret;
> @@ -106,15 +107,20 @@ static int fstatat_kstat(int fd, const char *restrict path, struct stat *restric
>
> return 0;
> }
> +#endif
>
> int fstatat(int fd, const char *restrict path, struct stat *restrict st, int flag)
> {
> int ret;
> +#ifdef SYS_fstatat
> if (sizeof((struct kstat){0}.st_atime_sec) < sizeof(time_t)) {
> ret = fstatat_statx(fd, path, st, flag);
> if (ret!=-ENOSYS) return __syscall_ret(ret);
> }
> ret = fstatat_kstat(fd, path, st, flag);
> +#else
> + ret = fstatat_statx(fd, path, st, flag);
> +#endif
> return __syscall_ret(ret);
> }
I'd like to have this streamlined more without duplicating the call,
but it's not really any big deal, just a little bit ugly, and I don't
see a better way to do it while preserving use of SYS_fstatat
everywhere it's sufficient.
> diff --git a/src/stdio/tempnam.c b/src/stdio/tempnam.c
> index 565df6b6..023ff422 100644
> --- a/src/stdio/tempnam.c
> +++ b/src/stdio/tempnam.c
> @@ -5,8 +5,10 @@
> #include <limits.h>
> #include <string.h>
> #include <stdlib.h>
> +#include <stdint.h>
> #include "syscall.h"
> #include "kstat.h"
> +#include "statx.h"
>
> #define MAXTRIES 100
>
> @@ -40,8 +42,13 @@ char *tempnam(const char *dir, const char *pfx)
> #ifdef SYS_lstat
> r = __syscall(SYS_lstat, s, &(struct kstat){0});
> #else
> +#ifdef SYS_fstatat
> r = __syscall(SYS_fstatat, AT_FDCWD, s,
> &(struct kstat){0}, AT_SYMLINK_NOFOLLOW);
> +#else
> + r = __syscall(SYS_statx, AT_FDCWD, s, AT_SYMLINK_NOFOLLOW, 0,
> + &(struct statx){0});
> +#endif
> #endif
Can be #elif. Also, is zero request mask ok here?
> diff --git a/src/stdio/tmpnam.c b/src/stdio/tmpnam.c
> index d667a836..c63dabcb 100644
> --- a/src/stdio/tmpnam.c
> +++ b/src/stdio/tmpnam.c
> @@ -4,8 +4,10 @@
> #include <sys/stat.h>
> #include <string.h>
> #include <stdlib.h>
> +#include <stdint.h>
> #include "syscall.h"
> #include "kstat.h"
> +#include "statx.h"
>
> #define MAXTRIES 100
>
> @@ -20,8 +22,13 @@ char *tmpnam(char *buf)
> #ifdef SYS_lstat
> r = __syscall(SYS_lstat, s, &(struct kstat){0});
> #else
> +#ifdef SYS_fstatat
> r = __syscall(SYS_fstatat, AT_FDCWD, s,
> &(struct kstat){0}, AT_SYMLINK_NOFOLLOW);
> +#else
> + r = __syscall(SYS_statx, AT_FDCWD, s, AT_SYMLINK_NOFOLLOW, 0,
> + &(struct statx){0});
> +#endif
> #endif
Same.
> if (r == -ENOENT) return strcpy(buf ? buf : internal, s);
> }
> diff --git a/src/time/__map_file.c b/src/time/__map_file.c
> index d3cefa82..f80752bc 100644
> --- a/src/time/__map_file.c
> +++ b/src/time/__map_file.c
> @@ -1,19 +1,30 @@
> +#define _BSD_SOURCE
> #include <sys/mman.h>
> #include <fcntl.h>
> #include <sys/stat.h>
> +#include <stdint.h>
> #include "syscall.h"
> #include "kstat.h"
> +#include "statx.h"
>
> const char unsigned *__map_file(const char *pathname, size_t *size)
> {
> - struct kstat st;
> const unsigned char *map = MAP_FAILED;
> int fd = sys_open(pathname, O_RDONLY|O_CLOEXEC|O_NONBLOCK);
> if (fd < 0) return 0;
> +#ifdef SYS_fstat
> + struct kstat st;
> if (!syscall(SYS_fstat, fd, &st)) {
> map = __mmap(0, st.st_size, PROT_READ, MAP_SHARED, fd, 0);
> *size = st.st_size;
> }
> +#else
> + struct statx st;
> + if (!syscall(SYS_statx, fd, "", AT_EMPTY_PATH, STATX_SIZE, &st)) {
> + map = __mmap(0, st.stx_size, PROT_READ, MAP_SHARED, fd, 0);
> + *size = st.stx_size;
> + }
> +#endif
As in fchmodat, etc., save the st[x]_size field in the #ifdef block
and get the common code out so it's not duplicated.
Rich
next prev parent reply other threads:[~2020-09-03 16:05 UTC|newest]
Thread overview: 31+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-09-03 11:22 [musl] [PATCH 00/14] riscv32 support Stefan O'Rear
2020-09-03 11:22 ` [musl] [PATCH 01/14] Remove ARMSUBARCH relic from configure Stefan O'Rear
2020-09-03 11:22 ` [musl] [PATCH 02/14] time64: Don't make aliases to nonexistent syscalls Stefan O'Rear
2020-09-03 15:56 ` Rich Felker
2020-09-03 19:36 ` Stefan O'Rear
2020-09-03 21:17 ` Rich Felker
2020-09-03 11:22 ` [musl] [PATCH 03/14] time64: Only getrlimit/setrlimit if they exist Stefan O'Rear
2020-09-03 11:22 ` [musl] [PATCH 04/14] time64: Only gettimeofday/settimeofday if exist Stefan O'Rear
2020-09-03 11:23 ` [musl] [PATCH 05/14] Add src/internal/statx.h Stefan O'Rear
2020-09-03 15:39 ` Arnd Bergmann
2020-09-03 15:51 ` Rich Felker
2020-09-03 18:08 ` Arnd Bergmann
2020-09-03 11:23 ` [musl] [PATCH 06/14] Only call fstatat if defined Stefan O'Rear
2020-09-03 16:05 ` Rich Felker [this message]
2020-09-04 1:47 ` Stefan O'Rear
2020-09-03 11:23 ` [musl] [PATCH 07/14] Emulate wait4 using waitid Stefan O'Rear
2020-09-03 14:56 ` Stefan O'Rear
2020-09-03 15:36 ` Arnd Bergmann
2020-09-03 15:40 ` Stefan O'Rear
2020-09-03 18:08 ` Arnd Bergmann
2020-09-03 15:49 ` Rich Felker
2020-09-03 16:25 ` Stefan O'Rear
2020-09-03 16:38 ` Rich Felker
2020-09-03 11:23 ` [musl] [PATCH 08/14] riscv: Fall back to syscall __riscv_flush_icache Stefan O'Rear
2020-09-03 11:23 ` [musl] [PATCH 09/14] riscv32: Target and subtarget detection Stefan O'Rear
2020-09-03 11:23 ` [musl] [PATCH 10/14] riscv32: add arch headers Stefan O'Rear
2020-09-03 15:49 ` Arnd Bergmann
2020-09-03 11:23 ` [musl] [PATCH 11/14] riscv32: Add fenv and math Stefan O'Rear
2020-09-03 11:23 ` [musl] [PATCH 12/14] riscv32: Add dlsym Stefan O'Rear
2020-09-03 11:23 ` [musl] [PATCH 13/14] riscv32: Add jmp_buf and sigreturn Stefan O'Rear
2020-09-03 11:23 ` [musl] [PATCH 14/14] riscv32: Add thread support Stefan O'Rear
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=20200903160502.GD3265@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).