Re: [PATCH] tools/nolibc: align signature of ioctl() with POSIX

[Willy Tarreau <w@1wt.eu>]

On Thu, Jan 23, 2025 at 02:24:13PM +0100, Thomas Weißschuh wrote:
> On 2025-01-23 14:16:25+0100, Willy Tarreau wrote:
> > Hi Thomas,
> > 
> > On Thu, Jan 23, 2025 at 02:04:29PM +0100, Thomas Weißschuh wrote:
> > > POSIX defines the signature of ioctl() as follows,
> > > to allow passing a pointer or integer without casting:
> > > 	int ioctl(int fildes, int request, ... /* arg */);
> > > 
> > > Nolibc ioctl() expects a pointer, forcing the user to manually cast.
> > > Using va_arg to make the signature more flexible would work but seems to
> > > prevent inlining of the function. Instead use a macro. "fd" and "req"
> > > will still be typechecked through sys_ioctl().
> > > 
> > > Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
> > > ---
> > >  tools/include/nolibc/sys.h | 8 ++------
> > >  1 file changed, 2 insertions(+), 6 deletions(-)
> > > 
> > > diff --git a/tools/include/nolibc/sys.h b/tools/include/nolibc/sys.h
> > > index d4a5c2399a66b200ebf7ab249569cce2285481a5..5cb2c66cc8cccc4d4a1126acfd0b30a6efc886c3 100644
> > > --- a/tools/include/nolibc/sys.h
> > > +++ b/tools/include/nolibc/sys.h
> > > @@ -532,7 +532,7 @@ uid_t getuid(void)
> > >  
> > >  
> > >  /*
> > > - * int ioctl(int fd, unsigned long req, void *value);
> > > + * int ioctl(int fd, unsigned long req, ... value);
> > >   */
> > >  
> > >  static __attribute__((unused))
> > > @@ -541,11 +541,7 @@ int sys_ioctl(int fd, unsigned long req, void *value)
> > >  	return my_syscall3(__NR_ioctl, fd, req, value);
> > >  }
> > >  
> > > -static __attribute__((unused))
> > > -int ioctl(int fd, unsigned long req, void *value)
> > > -{
> > > -	return __sysret(sys_ioctl(fd, req, value));
> > > -}
> > > +#define ioctl(fd, req, value) __sysret(sys_ioctl(fd, req, (void *)(value)))
> > 
> > You risk to get a warning about casting a pointer from an integer of
> > a different size if you pass an int there. I think should should perform
> > a double cast instead:
> > 
> >   #define ioctl(fd, req, value) __sysret(sys_ioctl(fd, req, (void *)(uintptr_t)(value)))
> > 
> > That way any int should cast fine, and pointers should as well.
> 
> I don't think this should ever happen. A warning there is actually
> useful. The POSIX signature forces users to pass something that is
> compatible with (void *), otherwise the vararg handling would be
> invalid.

I'm a bit confused because while my man page says:

   The third argument is an untyped pointer to memory.

what I'm finding in POSIX is:
 
   The type of arg depends upon the particular control request, but it shall
   be either an integer or a pointer to a device-specific data structure.

So I'm not seeing anything insisting on the int being the size of a
pointer. Even the FreeBSD man explicitly mentions char *, caddr_t or
int. Thus I do think that int is expressly permitted and might exist.
I mean, I don't care much, but if we try to fix something that requires
a cast, to end up with users having to write "(long)i" in their code,
we won't have made much progress :-/

Willy