Linux: Include <linux/sockios.h> in <bits/socket.h> under __USE_MISC
diff mbox series

Message ID 87ftmys3un.fsf@oldenburg2.str.redhat.com
State New
Headers show
Series
  • Linux: Include <linux/sockios.h> in <bits/socket.h> under __USE_MISC
Related show

Commit Message

Florian Weimer July 22, 2019, 11:31 a.m. UTC
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.

[[[
This is related to this thread:

From: Sergei Trofimovich <slyfox@gentoo.org>
Subject: linux-headers-5.2 and proper use of SIOCGSTAMP
To: netdev@vger.kernel.org, linux-kernel@vger.kernel.org, libc-alpha@sourceware.org
Cc: Arnd Bergmann <arnd@arndb.de>, "David S. Miller" <davem@davemloft.net>,
 mtk.manpages@gmail.com, linux-man@vger.kernel.org
Date: Sat, 20 Jul 2019 17:48:44 +0100 (1 day, 18 hours, 40 minutes ago)
Message-ID: <20190720174844.4b989d34@sf>

I have tried to verify this against our 3.10 kernel headers and the 5.2
headers, and I do not see any failures in glibc itself (the latter with
build-many-glibcs.py).  Impact on application code is unclear at this
point, of course.

This patch depends on the earlier Linux 5.2 compatibility patch which
introduced <bits/socket-constants.h>.
]]]

2019-07-22  Florian Weimer  <fweimer@redhat.com>

	* sysdeps/unix/sysv/linux/bits/socket.h [__USE_MISC]: Include
	<linux/sockios.h>.

Comments

Arnd Bergmann July 22, 2019, 11:34 a.m. UTC | #1
On Mon, Jul 22, 2019 at 1:31 PM Florian Weimer <fweimer@redhat.com> 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
Szabolcs Nagy July 22, 2019, 1:41 p.m. UTC | #2
On 22/07/2019 12:34, Arnd Bergmann wrote:
> On Mon, Jul 22, 2019 at 1:31 PM Florian Weimer <fweimer@redhat.com> 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())
Florian Weimer July 22, 2019, 1:44 p.m. UTC | #3
* 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
Szabolcs Nagy July 22, 2019, 1:47 p.m. UTC | #4
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.
Florian Weimer Aug. 15, 2019, 3:20 p.m. UTC | #5
* Florian Weimer:

> 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.
>
> [[[
> This is related to this thread:
>
> From: Sergei Trofimovich <slyfox@gentoo.org>
> Subject: linux-headers-5.2 and proper use of SIOCGSTAMP
> To: netdev@vger.kernel.org, linux-kernel@vger.kernel.org, libc-alpha@sourceware.org
> Cc: Arnd Bergmann <arnd@arndb.de>, "David S. Miller" <davem@davemloft.net>,
>  mtk.manpages@gmail.com, linux-man@vger.kernel.org
> Date: Sat, 20 Jul 2019 17:48:44 +0100 (1 day, 18 hours, 40 minutes ago)
> Message-ID: <20190720174844.4b989d34@sf>
>
> I have tried to verify this against our 3.10 kernel headers and the 5.2
> headers, and I do not see any failures in glibc itself (the latter with
> build-many-glibcs.py).  Impact on application code is unclear at this
> point, of course.
>
> This patch depends on the earlier Linux 5.2 compatibility patch which
> introduced <bits/socket-constants.h>.
> ]]]
>
> 2019-07-22  Florian Weimer  <fweimer@redhat.com>
>
> 	* sysdeps/unix/sysv/linux/bits/socket.h [__USE_MISC]: Include
> 	<linux/sockios.h>.
>
> diff --git a/sysdeps/unix/sysv/linux/bits/socket.h b/sysdeps/unix/sysv/linux/bits/socket.h
> index 082f8b9031..ff5b705f41 100644
> --- a/sysdeps/unix/sysv/linux/bits/socket.h
> +++ b/sysdeps/unix/sysv/linux/bits/socket.h
> @@ -352,6 +352,7 @@ struct ucred
>  #ifdef __USE_MISC
>  # include <bits/types/time_t.h>
>  # include <asm/socket.h>
> +# include <linux/sockios.h>
>  #else
>  # define SO_DEBUG 1
>  # include <bits/socket-constants.h>

I sort of droped the ball here. 8-/

Do we still want this change?  And perhaps backport it to 2.30 as well?

Thanks,
Florian

Patch
diff mbox series

diff --git a/sysdeps/unix/sysv/linux/bits/socket.h b/sysdeps/unix/sysv/linux/bits/socket.h
index 082f8b9031..ff5b705f41 100644
--- a/sysdeps/unix/sysv/linux/bits/socket.h
+++ b/sysdeps/unix/sysv/linux/bits/socket.h
@@ -352,6 +352,7 @@  struct ucred
 #ifdef __USE_MISC
 # include <bits/types/time_t.h>
 # include <asm/socket.h>
+# include <linux/sockios.h>
 #else
 # define SO_DEBUG 1
 # include <bits/socket-constants.h>