diff mbox series

[2/4] tools/nolibc: avoid unused parameter warnings for ENOSYS fallbacks

Message ID 20230914-nolibc-syscall-nr-v1-2-e50df410da11@weissschuh.net (mailing list archive)
State Superseded
Headers show
Series tools/nolibc: cleanups for syscall fallbacks | expand

Checks

Context Check Description
conchuod/cover_letter success Series has a cover letter
conchuod/tree_selection success Guessed tree name to be for-next at HEAD 0bb80ecc33a8
conchuod/fixes_present success Fixes tag not required for -next series
conchuod/maintainers_pattern success MAINTAINERS pattern errors before the patch: 5 and now 5
conchuod/verify_signedoff success Signed-off-by tag matches author and committer
conchuod/kdoc success Errors and warnings before: 0 this patch: 0
conchuod/build_rv64_clang_allmodconfig success Errors and warnings before: 9 this patch: 9
conchuod/module_param success Was 0 now: 0
conchuod/build_rv64_gcc_allmodconfig success Errors and warnings before: 9 this patch: 9
conchuod/build_rv32_defconfig success Build OK
conchuod/dtb_warn_rv64 success Errors and warnings before: 25 this patch: 25
conchuod/header_inline success No static functions without inline keyword in header files
conchuod/checkpatch success total: 0 errors, 0 warnings, 0 checks, 103 lines checked
conchuod/build_rv64_nommu_k210_defconfig success Build OK
conchuod/verify_fixes success No Fixes tag
conchuod/build_rv64_nommu_virt_defconfig success Build OK

Commit Message

Thomas Weißschuh Sept. 14, 2023, 4:01 p.m. UTC
The ENOSYS fallback code does not use its functions parameters.
This can lead to compiler warnings about unused parameters.

Explicitly avoid these warnings.

Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
---
 tools/include/nolibc/sys.h | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

Comments

Willy Tarreau Sept. 17, 2023, 2:58 a.m. UTC | #1
On Thu, Sep 14, 2023 at 06:01:18PM +0200, Thomas Weißschuh wrote:
> The ENOSYS fallback code does not use its functions parameters.
> This can lead to compiler warnings about unused parameters.
> 
> Explicitly avoid these warnings.

Just out of curiosity, did you find a valid case for enabling this
warning or were you trying various combinations ? I'm asking because
I've never seen it enabled anywhere given that it's probably the most 
useless and unusable warning: as soon as you're dealing with function
pointers, you start to have multiple functions with a similar
prototype, some of which just don't need certain arguments, and the
only way to shut the warning is to significantly uglify the code.

If really needed, I'm wondering if instead we shouldn't have an
"no_syscall*" set of macros, that would have the same signature
as my_syscall* to just consume all args in the same order and
return -ENOSYS. E.g, consider the following:

  @@ -934,6 +960,11 @@ int sys_select(int nfds, fd_set *rfds, fd_set *wfds, fd_set *efds, struct timeva
   #endif
   	return my_syscall5(__NR__newselect, nfds, rfds, wfds, efds, timeout);
   #else
  +	(void)nfds;
  +	(void)rfds;
  +	(void)wfds;
  +	(void)efds;
  +	(void)timeout;
   	return -ENOSYS;
   #endif

It would become:

  @@ -934,6 +960,11 @@ int sys_select(int nfds, fd_set *rfds, fd_set *wfds, fd_set *efds, struct timeva
   #endif
   	return my_syscall5(__NR__newselect, nfds, rfds, wfds, efds, timeout);
   #else
  +	return no_syscall5(nfds, rfds, wfds, efds, timeout);
  -	return -ENOSYS;
   #endif

What do you think ?

Thanks!
Willy
Thomas Weißschuh Sept. 17, 2023, 5:49 a.m. UTC | #2
On 2023-09-17 04:58:51+0200, Willy Tarreau wrote:
> On Thu, Sep 14, 2023 at 06:01:18PM +0200, Thomas Weißschuh wrote:
> > The ENOSYS fallback code does not use its functions parameters.
> > This can lead to compiler warnings about unused parameters.
> > 
> > Explicitly avoid these warnings.
> 
> Just out of curiosity, did you find a valid case for enabling this
> warning or were you trying various combinations ? I'm asking because
> I've never seen it enabled anywhere given that it's probably the most 
> useless and unusable warning: as soon as you're dealing with function
> pointers, you start to have multiple functions with a similar
> prototype, some of which just don't need certain arguments, and the
> only way to shut the warning is to significantly uglify the code.

nolibc-test uses it currently and I also used it in some projects.

> If really needed, I'm wondering if instead we shouldn't have an
> "no_syscall*" set of macros, that would have the same signature
> as my_syscall* to just consume all args in the same order and
> return -ENOSYS. E.g, consider the following:
> 
>   @@ -934,6 +960,11 @@ int sys_select(int nfds, fd_set *rfds, fd_set *wfds, fd_set *efds, struct timeva
>    #endif
>    	return my_syscall5(__NR__newselect, nfds, rfds, wfds, efds, timeout);
>    #else
>   +	(void)nfds;
>   +	(void)rfds;
>   +	(void)wfds;
>   +	(void)efds;
>   +	(void)timeout;
>    	return -ENOSYS;
>    #endif
> 
> It would become:
> 
>   @@ -934,6 +960,11 @@ int sys_select(int nfds, fd_set *rfds, fd_set *wfds, fd_set *efds, struct timeva
>    #endif
>    	return my_syscall5(__NR__newselect, nfds, rfds, wfds, efds, timeout);
>    #else
>   +	return no_syscall5(nfds, rfds, wfds, efds, timeout);
>   -	return -ENOSYS;
>    #endif
> 
> What do you think ?

The idea sounds good. But "no_syscall5" sounds a bit non-obvious to me.

Maybe the macro-equivalent of this?

static inline int __nolibc_enosys(...)
{
	return -ENOSYS;
}

The only-vararg function unfortunately needs C23 so we can't use it.

It's clear to the users that this is about ENOSYS and we don't need a
bunch of new macros similar.

I'll check if it is cleaner to implement a generic macro or a few
numbered ones.


Thomas
Willy Tarreau Sept. 17, 2023, 9:48 a.m. UTC | #3
On Sun, Sep 17, 2023 at 07:49:57AM +0200, Thomas Weißschuh wrote:
> On 2023-09-17 04:58:51+0200, Willy Tarreau wrote:
> > On Thu, Sep 14, 2023 at 06:01:18PM +0200, Thomas Weißschuh wrote:
> > > The ENOSYS fallback code does not use its functions parameters.
> > > This can lead to compiler warnings about unused parameters.
> > > 
> > > Explicitly avoid these warnings.
> > 
> > Just out of curiosity, did you find a valid case for enabling this
> > warning or were you trying various combinations ? I'm asking because
> > I've never seen it enabled anywhere given that it's probably the most 
> > useless and unusable warning: as soon as you're dealing with function
> > pointers, you start to have multiple functions with a similar
> > prototype, some of which just don't need certain arguments, and the
> > only way to shut the warning is to significantly uglify the code.
> 
> nolibc-test uses it currently and I also used it in some projects.

OK then let's handle it.

> >   @@ -934,6 +960,11 @@ int sys_select(int nfds, fd_set *rfds, fd_set *wfds, fd_set *efds, struct timeva
> >    #endif
> >    	return my_syscall5(__NR__newselect, nfds, rfds, wfds, efds, timeout);
> >    #else
> >   +	return no_syscall5(nfds, rfds, wfds, efds, timeout);
> >   -	return -ENOSYS;
> >    #endif
> > 
> > What do you think ?
> 
> The idea sounds good. But "no_syscall5" sounds a bit non-obvious to me.

Of course, I was just trying to illustrate. I'm never good at giving
names.

> Maybe the macro-equivalent of this?
> 
> static inline int __nolibc_enosys(...)
> {
> 	return -ENOSYS;
> }
> 
> The only-vararg function unfortunately needs C23 so we can't use it.
>
> It's clear to the users that this is about ENOSYS and we don't need a
> bunch of new macros similar.

I like it, I didn't think about varargs, it's an excellent idea! Let's
just do simpler, start with a first arg "syscall_num" that we may later
reuse for debugging, and just mark this one unused:

  static inline int __nolibc_enosys(int syscall_num, ...)
  {
	(void)syscall_num;
  	return -ENOSYS;
  }

Willy
Thomas Weißschuh Sept. 17, 2023, 3:07 p.m. UTC | #4
On 2023-09-17 11:48:27+0200, Willy Tarreau wrote:
> [..]
> > Maybe the macro-equivalent of this?
> > 
> > static inline int __nolibc_enosys(...)
> > {
> > 	return -ENOSYS;
> > }
> > 
> > The only-vararg function unfortunately needs C23 so we can't use it.
> >
> > It's clear to the users that this is about ENOSYS and we don't need a
> > bunch of new macros similar.
> 
> I like it, I didn't think about varargs, it's an excellent idea! Let's
> just do simpler, start with a first arg "syscall_num" that we may later
> reuse for debugging, and just mark this one unused:
> 
>   static inline int __nolibc_enosys(int syscall_num, ...)
>   {
> 	(void)syscall_num;
>   	return -ENOSYS;
>   }

But which syscall_num to use, as the point of __nolibc_enosys() would be
that no syscall number is available and the defines are missing.

For debugging we could add a string argument, though.
Willy Tarreau Sept. 17, 2023, 3:08 p.m. UTC | #5
On Sun, Sep 17, 2023 at 05:07:18PM +0200, Thomas Weißschuh wrote:
> On 2023-09-17 11:48:27+0200, Willy Tarreau wrote:
> > [..]
> > > Maybe the macro-equivalent of this?
> > > 
> > > static inline int __nolibc_enosys(...)
> > > {
> > > 	return -ENOSYS;
> > > }
> > > 
> > > The only-vararg function unfortunately needs C23 so we can't use it.
> > >
> > > It's clear to the users that this is about ENOSYS and we don't need a
> > > bunch of new macros similar.
> > 
> > I like it, I didn't think about varargs, it's an excellent idea! Let's
> > just do simpler, start with a first arg "syscall_num" that we may later
> > reuse for debugging, and just mark this one unused:
> > 
> >   static inline int __nolibc_enosys(int syscall_num, ...)
> >   {
> > 	(void)syscall_num;
> >   	return -ENOSYS;
> >   }
> 
> But which syscall_num to use, as the point of __nolibc_enosys() would be
> that no syscall number is available and the defines are missing.

good point :-)

> For debugging we could add a string argument, though.

That works for me.

Willy
diff mbox series

Patch

diff --git a/tools/include/nolibc/sys.h b/tools/include/nolibc/sys.h
index b478750c9004..bc56310c6bdf 100644
--- a/tools/include/nolibc/sys.h
+++ b/tools/include/nolibc/sys.h
@@ -133,6 +133,8 @@  int sys_chmod(const char *path, mode_t mode)
 #elif defined(__NR_chmod)
 	return my_syscall2(__NR_chmod, path, mode);
 #else
+	(void)path;
+	(void)mode;
 	return -ENOSYS;
 #endif
 }
@@ -156,6 +158,9 @@  int sys_chown(const char *path, uid_t owner, gid_t group)
 #elif defined(__NR_chown)
 	return my_syscall3(__NR_chown, path, owner, group);
 #else
+	(void)path;
+	(void)owner;
+	(void)group;
 	return -ENOSYS;
 #endif
 }
@@ -230,6 +235,8 @@  int sys_dup2(int old, int new)
 #elif defined(__NR_dup2)
 	return my_syscall2(__NR_dup2, old, new);
 #else
+	(void)old;
+	(void)new;
 	return -ENOSYS;
 #endif
 }
@@ -486,6 +493,8 @@  int sys_gettimeofday(struct timeval *tv, struct timezone *tz)
 #ifdef __NR_gettimeofday
 	return my_syscall2(__NR_gettimeofday, tv, tz);
 #else
+	(void)tv;
+	(void)tz;
 	return -ENOSYS;
 #endif
 }
@@ -563,6 +572,8 @@  int sys_link(const char *old, const char *new)
 #elif defined(__NR_link)
 	return my_syscall2(__NR_link, old, new);
 #else
+	(void)old;
+	(void)new;
 	return -ENOSYS;
 #endif
 }
@@ -584,6 +595,9 @@  off_t sys_lseek(int fd, off_t offset, int whence)
 #ifdef __NR_lseek
 	return my_syscall3(__NR_lseek, fd, offset, whence);
 #else
+	(void)fd;
+	(void)offset;
+	(void)whence;
 	return -ENOSYS;
 #endif
 }
@@ -607,6 +621,8 @@  int sys_mkdir(const char *path, mode_t mode)
 #elif defined(__NR_mkdir)
 	return my_syscall2(__NR_mkdir, path, mode);
 #else
+	(void)path;
+	(void)mode;
 	return -ENOSYS;
 #endif
 }
@@ -629,6 +645,7 @@  int sys_rmdir(const char *path)
 #elif defined(__NR_unlinkat)
 	return my_syscall3(__NR_unlinkat, AT_FDCWD, path, AT_REMOVEDIR);
 #else
+	(void)path;
 	return -ENOSYS;
 #endif
 }
@@ -652,6 +669,9 @@  long sys_mknod(const char *path, mode_t mode, dev_t dev)
 #elif defined(__NR_mknod)
 	return my_syscall3(__NR_mknod, path, mode, dev);
 #else
+	(void)path;
+	(void)mode;
+	(void)dev;
 	return -ENOSYS;
 #endif
 }
@@ -742,6 +762,9 @@  int sys_open(const char *path, int flags, mode_t mode)
 #elif defined(__NR_open)
 	return my_syscall3(__NR_open, path, flags, mode);
 #else
+	(void)path;
+	(void)flags;
+	(void)mode;
 	return -ENOSYS;
 #endif
 }
@@ -842,6 +865,9 @@  int sys_poll(struct pollfd *fds, int nfds, int timeout)
 #elif defined(__NR_poll)
 	return my_syscall3(__NR_poll, fds, nfds, timeout);
 #else
+	(void)fds;
+	(void)nfds;
+	(void)timeout;
 	return -ENOSYS;
 #endif
 }
@@ -934,6 +960,11 @@  int sys_select(int nfds, fd_set *rfds, fd_set *wfds, fd_set *efds, struct timeva
 #endif
 	return my_syscall5(__NR__newselect, nfds, rfds, wfds, efds, timeout);
 #else
+	(void)nfds;
+	(void)rfds;
+	(void)wfds;
+	(void)efds;
+	(void)timeout;
 	return -ENOSYS;
 #endif
 }