[v3,1/3] tools/nolibc: fix up #error compile failures with -ENOSYS

Compiling nolibc for rv32 got such errors:

    In file included from nolibc/sysroot/riscv/include/nolibc.h:99,
                     from nolibc/sysroot/riscv/include/errno.h:26,
                     from nolibc/sysroot/riscv/include/stdio.h:14,
                     from tools/testing/selftests/nolibc/nolibc-test.c:12:
    nolibc/sysroot/riscv/include/sys.h:946:2: error: #error Neither __NR_ppoll nor __NR_poll defined, cannot implement sys_poll()
      946 | #error Neither __NR_ppoll nor __NR_poll defined, cannot implement sys_poll()
          |  ^~~~~
    nolibc/sysroot/riscv/include/sys.h:1062:2: error: #error None of __NR_select, __NR_pselect6, nor __NR__newselect defined, cannot implement sys_select()
     1062 | #error None of __NR_select, __NR_pselect6, nor __NR__newselect defined, cannot implement sys_select()

If a syscall is not supported by a target platform, 'return -ENOSYS' is
better than '#error', which lets the other syscalls work as-is and
allows developers to fix up the test failures reported by nolibc-test
one by one later.

This converts all of the '#error' to 'return -ENOSYS', so, all of the
'#error' failures are fixed.

Suggested-by: Arnd Bergmann <arnd@arndb.de>
Link: https://lore.kernel.org/linux-riscv/5e7d2adf-e96f-41ca-a4c6-5c87a25d4c9c@app.fastmail.com/
Signed-off-by: Zhangjin Wu <falcon@tinylab.org>
---
 tools/include/nolibc/sys.h | 26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)

On Sat, Jun 3, 2023, at 11:01, Zhangjin Wu wrote:
>
> Suggested-by: Arnd Bergmann <arnd@arndb.de>
> Link: 
> https://lore.kernel.org/linux-riscv/5e7d2adf-e96f-41ca-a4c6-5c87a25d4c9c@app.fastmail.com/
> Signed-off-by: Zhangjin Wu <falcon@tinylab.org>
> ---
>  tools/include/nolibc/sys.h | 26 +++++++++++++-------------
>  1 file changed, 13 insertions(+), 13 deletions(-)
>
> diff --git a/tools/include/nolibc/sys.h b/tools/include/nolibc/sys.h
> index 856249a11890..78c86f124335 100644
> --- a/tools/include/nolibc/sys.h
> +++ b/tools/include/nolibc/sys.h
> @@ -124,7 +124,7 @@ int sys_chmod(const char *path, mode_t mode)
>  #elif defined(__NR_chmod)
>  	return my_syscall2(__NR_chmod, path, mode);
>  #else
> -#error Neither __NR_fchmodat nor __NR_chmod defined, cannot implement 
> sys_chmod()
> +	return -ENOSYS;
>  #endif
>  }

I think the most logical would be to have each syscall (chmod,
fchmodat, ...) have its own function that returns -ENOSYS if
that is not defined, and have the logic that decides which one
to use as a separate function.

This patch is a step in that direction though, so I think that's
totally fine.

     Arnd

> On Sat, Jun 3, 2023, at 11:01, Zhangjin Wu wrote:
> >
> > Suggested-by: Arnd Bergmann <arnd@arndb.de>
> > Link:
> > https://lore.kernel.org/linux-riscv/5e7d2adf-e96f-41ca-a4c6-5c87a25d4c9c@app.fastmail.com/
> > Signed-off-by: Zhangjin Wu <falcon@tinylab.org>
> > ---
> >  tools/include/nolibc/sys.h | 26 +++++++++++++-------------
> >  1 file changed, 13 insertions(+), 13 deletions(-)
> >
> > diff --git a/tools/include/nolibc/sys.h b/tools/include/nolibc/sys.h
> > index 856249a11890..78c86f124335 100644
> > --- a/tools/include/nolibc/sys.h
> > +++ b/tools/include/nolibc/sys.h
> > @@ -124,7 +124,7 @@ int sys_chmod(const char *path, mode_t mode)
> >  #elif defined(__NR_chmod)
> >  	return my_syscall2(__NR_chmod, path, mode);
> >  #else
> > -#error Neither __NR_fchmodat nor __NR_chmod defined, cannot implement
> > sys_chmod()
> > +	return -ENOSYS;
> >  #endif
> >  }
>
> I think the most logical would be to have each syscall (chmod,
> fchmodat, ...) have its own function that returns -ENOSYS if
> that is not defined, and have the logic that decides which one
> to use as a separate function.
>

Yeah, agreed, we can clean up them one by one, If split them to their own
syscalls, I have two questions (for Arnd, and Willy too):

1. do we need to add the corresponding library routines at the same time?

  Use llseek() as an example, there will be llseek() and lsee64(). If off_t
  would be converted to 64bit, then, they can be simply added as follows:

    #define lseek64 lseek
    #define llseek lseek

  Or something like this:

    static __attribute__((unused))
    loff_t lseek(int fd, loff_t offset, int whence)
    {
    	return lseek(fd, offset, whence);
    }

    static __attribute__((unused))
    off64_t lseek(int fd, off64_t offset, int whence)
    {
    	return lseek(fd, offset, whence);
    }

  This one aligns with the other already added library routines.

  Which one do you like more?

2. If so, how to cope with the new types when add the library routines?

  Still use the above llseek() as an example, If not use '#define' method,
  We may need to declare loff_t and off64_t in std.h too:

    #define off64_t off_t
    #define loff_t off_t

  Or align with the other new types, use 'typedef' instead of '#define'.

  And further, use poll() as an example, in its manpage [1], there may be some
  new types, such as 'nfds_t', but 'int' is used in tools/include/nolibc/sys.h
  currently, do we need to add nfds_t?

  The 'idtypes_t' and 'id_t' types used by waitid() [2] is similar, both
  of them can simply use the 'int' type.

The above two questions are important to the coming patches, it may determine
how I should tune the new llseek() and waitid() syscalls and their library
routines. very welcome your suggestions.

> This patch is a step in that direction though, so I think that's
> totally fine.

Thanks, so, can I pick your Reviewed-by for the first two patches? I'm ready to
send v4 now ;-)

Best regards,
Zhangjin

---
[1]: https://linux.die.net/man/2/poll
[2]: https://linux.die.net/man/2/waitid

>
>      Arnd

On Wed, Jun 7, 2023, at 07:19, Zhangjin Wu wrote:
>> On Sat, Jun 3, 2023, at 11:01, Zhangjin Wu wrote:
>
> Yeah, agreed, we can clean up them one by one, If split them to their own
> syscalls, I have two questions (for Arnd, and Willy too):
>
> 1. do we need to add the corresponding library routines at the same time?
>
>   Use llseek() as an example, there will be llseek() and lsee64(). If off_t
>   would be converted to 64bit, then, they can be simply added as follows:
>
>     #define lseek64 lseek
>     #define llseek lseek
>
>   Or something like this:
>
>     static __attribute__((unused))
>     loff_t lseek(int fd, loff_t offset, int whence)
>     {
>     	return lseek(fd, offset, whence);
>     }
>
>     static __attribute__((unused))
>     off64_t lseek(int fd, off64_t offset, int whence)
>     {
>     	return lseek(fd, offset, whence);
>     }
>
>   This one aligns with the other already added library routines.
>
>   Which one do you like more?

lseek() is probably not a good example, as the llseek and lseek64
library functions are both nonstandard, and I'd suggest leaving them
out of nolibc altogether.

Are there any examples of functions where we actually want mulitple
versions?

> 2. If so, how to cope with the new types when add the library routines?
>
>   Still use the above llseek() as an example, If not use '#define' method,
>   We may need to declare loff_t and off64_t in std.h too:
>
>     #define off64_t off_t
>     #define loff_t off_t
>
>   Or align with the other new types, use 'typedef' instead of '#define'.

If we do want to include the explicit off64_t interfaces from glibc,
I'd suggest doing it the same way as musl:
https://git.musl-libc.org/cgit/musl/tree/include/unistd.h#n201

>
>> This patch is a step in that direction though, so I think that's
>> totally fine.
>
> Thanks, so, can I pick your Reviewed-by for the first two patches? I'm ready to
> send v4 now ;-)

Yes, please do.

     Arnd