[PATCH] syscalls: Update the syscall #defines to match uapi

[PATCH] syscalls: Update the syscall #defines to match uapi

[Alistair Francis]

Update the #defines around sys_fstat64() and sys_fstatat64() to match
the #defines around the __NR3264_fstatat and __NR3264_fstat definitions
in include/uapi/asm-generic/unistd.h. This avoids compiler failures if
one is defined.

Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
---
 include/linux/syscalls.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
index 2bcef4c70183..e4bf5e480d60 100644
--- a/include/linux/syscalls.h
+++ b/include/linux/syscalls.h
@@ -512,7 +512,7 @@ asmlinkage long sys_readlinkat(int dfd, const char __user *path, char __user *bu
 asmlinkage long sys_newfstatat(int dfd, const char __user *filename,
 			       struct stat __user *statbuf, int flag);
 asmlinkage long sys_newfstat(unsigned int fd, struct stat __user *statbuf);
-#if defined(__ARCH_WANT_STAT64) || defined(__ARCH_WANT_COMPAT_STAT64)
+#if defined(__ARCH_WANT_NEW_STAT) || defined(__ARCH_WANT_STAT64)
 asmlinkage long sys_fstat64(unsigned long fd, struct stat64 __user *statbuf);
 asmlinkage long sys_fstatat64(int dfd, const char __user *filename,
 			       struct stat64 __user *statbuf, int flag);
-- 
2.22.0

Re: [PATCH] syscalls: Update the syscall #defines to match uapi

[Andy Lutomirski]

On Fri, Aug 9, 2019 at 6:11 PM Alistair Francis
<alistair.francis@wdc.com> wrote:
>
> Update the #defines around sys_fstat64() and sys_fstatat64() to match
> the #defines around the __NR3264_fstatat and __NR3264_fstat definitions
> in include/uapi/asm-generic/unistd.h. This avoids compiler failures if
> one is defined.

What do you mean by "if one is defined"?

The patch seems like it's probably okay, but I can't understand the changelog.

--Andy

Re: [PATCH] syscalls: Update the syscall #defines to match uapi

[Alistair Francis]

On Sun, Aug 11, 2019 at 5:00 PM Andy Lutomirski <luto@kernel.org> wrote:
>
> On Fri, Aug 9, 2019 at 6:11 PM Alistair Francis
> <alistair.francis@wdc.com> wrote:
> >
> > Update the #defines around sys_fstat64() and sys_fstatat64() to match
> > the #defines around the __NR3264_fstatat and __NR3264_fstat definitions
> > in include/uapi/asm-generic/unistd.h. This avoids compiler failures if
> > one is defined.
>
> What do you mean by "if one is defined"?

Yeah, I guess that isn't clear. What I mean is that if
__ARCH_WANT_NEW_STAT is defined but __ARCH_WANT_STAT64 isn't currently
it will fail to build.

>
> The patch seems like it's probably okay, but I can't understand the changelog.

I can send a v2 with an updated commit message.

Alistair

>
> --Andy

Re: [PATCH] syscalls: Update the syscall #defines to match uapi

[Arnd Bergmann]

On Sat, Aug 10, 2019 at 3:11 AM Alistair Francis
<alistair.francis@wdc.com> wrote:
>
> Update the #defines around sys_fstat64() and sys_fstatat64() to match
> the #defines around the __NR3264_fstatat and __NR3264_fstat definitions
> in include/uapi/asm-generic/unistd.h. This avoids compiler failures if
> one is defined.

What is the compiler failure you get?

> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> ---
>  include/linux/syscalls.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
> index 2bcef4c70183..e4bf5e480d60 100644
> --- a/include/linux/syscalls.h
> +++ b/include/linux/syscalls.h
> @@ -512,7 +512,7 @@ asmlinkage long sys_readlinkat(int dfd, const char __user *path, char __user *bu
>  asmlinkage long sys_newfstatat(int dfd, const char __user *filename,
>                                struct stat __user *statbuf, int flag);
>  asmlinkage long sys_newfstat(unsigned int fd, struct stat __user *statbuf);
> -#if defined(__ARCH_WANT_STAT64) || defined(__ARCH_WANT_COMPAT_STAT64)
> +#if defined(__ARCH_WANT_NEW_STAT) || defined(__ARCH_WANT_STAT64)
>  asmlinkage long sys_fstat64(unsigned long fd, struct stat64 __user *statbuf);
>  asmlinkage long sys_fstatat64(int dfd, const char __user *filename,
>                                struct stat64 __user *statbuf, int flag);

I think this is wrong: when __ARCH_WANT_NEW_STAT is set, we are
on a 64-bit architecture and only want the sys_newfstat{,at} system
calls, not sys_fstat{,at}64 that gets used on 32-bit machines.

The #if check in the syscalls.h file also matches the definition of
the function.

       Arnd

Re: [PATCH] syscalls: Update the syscall #defines to match uapi

[Alistair Francis]

On Mon, Aug 12, 2019 at 2:49 AM Arnd Bergmann <arnd@arndb.de> wrote:
>
> On Sat, Aug 10, 2019 at 3:11 AM Alistair Francis
> <alistair.francis@wdc.com> wrote:
> >
> > Update the #defines around sys_fstat64() and sys_fstatat64() to match
> > the #defines around the __NR3264_fstatat and __NR3264_fstat definitions
> > in include/uapi/asm-generic/unistd.h. This avoids compiler failures if
> > one is defined.
>
> What is the compiler failure you get?

I don't have it infornt of me but it was along the lines of
sys_fstat64/sys_fstatat64 not being defined when __ARCH_WANT_NEW_STAT
is defined but __ARCH_WANT_STAT64 isn't.

>
> > Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> > ---
> >  include/linux/syscalls.h | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
> > index 2bcef4c70183..e4bf5e480d60 100644
> > --- a/include/linux/syscalls.h
> > +++ b/include/linux/syscalls.h
> > @@ -512,7 +512,7 @@ asmlinkage long sys_readlinkat(int dfd, const char __user *path, char __user *bu
> >  asmlinkage long sys_newfstatat(int dfd, const char __user *filename,
> >                                struct stat __user *statbuf, int flag);
> >  asmlinkage long sys_newfstat(unsigned int fd, struct stat __user *statbuf);
> > -#if defined(__ARCH_WANT_STAT64) || defined(__ARCH_WANT_COMPAT_STAT64)
> > +#if defined(__ARCH_WANT_NEW_STAT) || defined(__ARCH_WANT_STAT64)
> >  asmlinkage long sys_fstat64(unsigned long fd, struct stat64 __user *statbuf);
> >  asmlinkage long sys_fstatat64(int dfd, const char __user *filename,
> >                                struct stat64 __user *statbuf, int flag);
>
> I think this is wrong: when __ARCH_WANT_NEW_STAT is set, we are
> on a 64-bit architecture and only want the sys_newfstat{,at} system
> calls, not sys_fstat{,at}64 that gets used on 32-bit machines.

Ah, that would make sense then. I don't think you will see the error then.

Alistair

>
> The #if check in the syscalls.h file also matches the definition of
> the function.
>
>        Arnd

Re: [PATCH] syscalls: Update the syscall #defines to match uapi

[Arnd Bergmann]

On Tue, Aug 13, 2019 at 9:01 PM Alistair Francis <alistair23@gmail.com> wrote:
> On Mon, Aug 12, 2019 at 2:49 AM Arnd Bergmann <arnd@arndb.de> wrote:

> > > diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
> > > index 2bcef4c70183..e4bf5e480d60 100644
> > > --- a/include/linux/syscalls.h
> > > +++ b/include/linux/syscalls.h
> > > @@ -512,7 +512,7 @@ asmlinkage long sys_readlinkat(int dfd, const char __user *path, char __user *bu
> > >  asmlinkage long sys_newfstatat(int dfd, const char __user *filename,
> > >                                struct stat __user *statbuf, int flag);
> > >  asmlinkage long sys_newfstat(unsigned int fd, struct stat __user *statbuf);
> > > -#if defined(__ARCH_WANT_STAT64) || defined(__ARCH_WANT_COMPAT_STAT64)
> > > +#if defined(__ARCH_WANT_NEW_STAT) || defined(__ARCH_WANT_STAT64)
> > >  asmlinkage long sys_fstat64(unsigned long fd, struct stat64 __user *statbuf);
> > >  asmlinkage long sys_fstatat64(int dfd, const char __user *filename,
> > >                                struct stat64 __user *statbuf, int flag);
> >
> > I think this is wrong: when __ARCH_WANT_NEW_STAT is set, we are
> > on a 64-bit architecture and only want the sys_newfstat{,at} system
> > calls, not sys_fstat{,at}64 that gets used on 32-bit machines.
>
> Ah, that would make sense then. I don't think you will see the error then.

So we don't need this patch to build riscv32 kernels, right? It's possible
that it was the result of an incorrect forward port of some other patch,
as older riscv32 kernels did provide stat64(), but newer ones only have
statx().

       Arnd

Re: [PATCH] syscalls: Update the syscall #defines to match uapi

[Alistair Francis]

On Tue, Aug 13, 2019 at 12:41 PM Arnd Bergmann <arnd@arndb.de> wrote:
>
> On Tue, Aug 13, 2019 at 9:01 PM Alistair Francis <alistair23@gmail.com> wrote:
> > On Mon, Aug 12, 2019 at 2:49 AM Arnd Bergmann <arnd@arndb.de> wrote:
>
> > > > diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
> > > > index 2bcef4c70183..e4bf5e480d60 100644
> > > > --- a/include/linux/syscalls.h
> > > > +++ b/include/linux/syscalls.h
> > > > @@ -512,7 +512,7 @@ asmlinkage long sys_readlinkat(int dfd, const char __user *path, char __user *bu
> > > >  asmlinkage long sys_newfstatat(int dfd, const char __user *filename,
> > > >                                struct stat __user *statbuf, int flag);
> > > >  asmlinkage long sys_newfstat(unsigned int fd, struct stat __user *statbuf);
> > > > -#if defined(__ARCH_WANT_STAT64) || defined(__ARCH_WANT_COMPAT_STAT64)
> > > > +#if defined(__ARCH_WANT_NEW_STAT) || defined(__ARCH_WANT_STAT64)
> > > >  asmlinkage long sys_fstat64(unsigned long fd, struct stat64 __user *statbuf);
> > > >  asmlinkage long sys_fstatat64(int dfd, const char __user *filename,
> > > >                                struct stat64 __user *statbuf, int flag);
> > >
> > > I think this is wrong: when __ARCH_WANT_NEW_STAT is set, we are
> > > on a 64-bit architecture and only want the sys_newfstat{,at} system
> > > calls, not sys_fstat{,at}64 that gets used on 32-bit machines.
> >
> > Ah, that would make sense then. I don't think you will see the error then.
>
> So we don't need this patch to build riscv32 kernels, right? It's possible
> that it was the result of an incorrect forward port of some other patch,
> as older riscv32 kernels did provide stat64(), but newer ones only have
> statx().

The issue came up when I was just changing some things for testing and
I thought it was a bug that others might run into. It isn't directly
related to the riscv32 kernel.

Alistair

>
>        Arnd