[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Thu, 3 Sep 2020 12:05:02 -0400
From: Rich Felker <dalias@...c.org>
To: musl@...ts.openwall.com
Subject: Re: [PATCH 06/14] Only call fstatat if defined
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
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.
