From mboxrd@z Thu Jan 1 00:00:00 1970 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on inbox.vuxu.org X-Spam-Level: X-Spam-Status: No, score=-3.3 required=5.0 tests=MAILING_LIST_MULTI, RCVD_IN_DNSWL_MED,RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL autolearn=ham autolearn_force=no version=3.4.4 Received: (qmail 3326 invoked from network); 8 Sep 2020 01:55:11 -0000 Received: from mother.openwall.net (195.42.179.200) by inbox.vuxu.org with ESMTPUTF8; 8 Sep 2020 01:55:11 -0000 Received: (qmail 30300 invoked by uid 550); 8 Sep 2020 01:55:09 -0000 Mailing-List: contact musl-help@lists.openwall.com; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: List-ID: Reply-To: musl@lists.openwall.com Received: (qmail 30279 invoked from network); 8 Sep 2020 01:55:08 -0000 Date: Mon, 7 Sep 2020 21:54:56 -0400 From: Rich Felker To: musl@lists.openwall.com Message-ID: <20200908015456.GT3265@brightrain.aerifal.cx> References: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) Subject: Re: [musl] riscv32 v2 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 > 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 > #include > #include > +#include > #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