2019-07-22 12:45:26

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH glibc] Linux: Include <linux/sockios.h> in <bits/socket.h> under __USE_MISC

On Mon, Jul 22, 2019 at 1:31 PM Florian Weimer <[email protected]> wrote:
>
> Historically, <asm/socket.h> (which is included from <bits/socket.h>)
> provided ioctl operations for sockets. User code accessed them
> through <sys/socket.h>. The kernel UAPI headers have removed these
> definitions in favor of <linux/sockios.h>. This commit makes them
> available via <sys/socket.h> again.

Looks good to me.

I wonder if we should still do these two changes in the kernel:

- include asm/socket.h from linux/socket.h for consistency
- move the defines that got moved from asm/sockios.h to linux/sockios.h
back to the previous location to help anyone who is user
newer kernel headers with older glibc headers.

Arnd


2019-07-22 13:44:14

by Szabolcs Nagy

[permalink] [raw]
Subject: Re: [PATCH glibc] Linux: Include <linux/sockios.h> in <bits/socket.h> under __USE_MISC

On 22/07/2019 12:34, Arnd Bergmann wrote:
> On Mon, Jul 22, 2019 at 1:31 PM Florian Weimer <[email protected]> wrote:
>>
>> Historically, <asm/socket.h> (which is included from <bits/socket.h>)
>> provided ioctl operations for sockets. User code accessed them
>> through <sys/socket.h>. The kernel UAPI headers have removed these
>> definitions in favor of <linux/sockios.h>. This commit makes them
>> available via <sys/socket.h> again.
>
> Looks good to me.
>
> I wonder if we should still do these two changes in the kernel:
>
> - include asm/socket.h from linux/socket.h for consistency
> - move the defines that got moved from asm/sockios.h to linux/sockios.h
> back to the previous location to help anyone who is user
> newer kernel headers with older glibc headers.

does user code actually expect these in sys/socket.h
or in asm/socket.h ?

man 7 socket
mentions SIOCGSTAMP but does not say the header.

man 2 ioctl_list
specifies include/asm-i386/socket.h as the location.

if user code tends to directly include asm/socket.h
then i think it's better to undo the kernel header
change than to put things in sys/socket.h.

(note: in musl these ioctl macros are in sys/ioctl.h
which is not a posix header so namespace rules are
less strict than for sys/socket.h and users tend to
include it for ioctl())

2019-07-22 13:45:26

by Florian Weimer

[permalink] [raw]
Subject: Re: [PATCH glibc] Linux: Include <linux/sockios.h> in <bits/socket.h> under __USE_MISC

* Szabolcs Nagy:

> (note: in musl these ioctl macros are in sys/ioctl.h
> which is not a posix header so namespace rules are
> less strict than for sys/socket.h and users tend to
> include it for ioctl())

<sys/ioctl.h> can be confusing because some of the constants may depend
on types that aren't declared by including the header. This makes their
macros unusable. Defining ioctl constants in headers which also provide
the matching types avoids that problem at least, also it can introduce
namespace issues.

Thanks,
Florian

2019-07-22 16:42:28

by Szabolcs Nagy

[permalink] [raw]
Subject: Re: [PATCH glibc] Linux: Include <linux/sockios.h> in <bits/socket.h> under __USE_MISC

On 22/07/2019 14:44, Florian Weimer wrote:
> * Szabolcs Nagy:
>
>> (note: in musl these ioctl macros are in sys/ioctl.h
>> which is not a posix header so namespace rules are
>> less strict than for sys/socket.h and users tend to
>> include it for ioctl())
>
> <sys/ioctl.h> can be confusing because some of the constants may depend
> on types that aren't declared by including the header. This makes their
> macros unusable. Defining ioctl constants in headers which also provide
> the matching types avoids that problem at least, also it can introduce
> namespace issues.

yeah, the way we deal with that is we hard code the numbers
since the size of struct is abi, they cannot change.