diff mbox

struct sockaddr_storage: Rename internal members

Message ID 20170901161651.D5F51439942E3@oldenburg.str.redhat.com
State New, archived
Headers

Commit Message

Florian Weimer Sept. 1, 2017, 4:16 p.m. UTC 
  In commit 3375cfafa7961c6ae0e509c31c3b3cef9ad1f03d (Make padding in
struct sockaddr_storage explicit), the offsets of the members
changed.  Some broken applications use these members to find
the start of the address data in struct sockaddr_in or
struct sockaddr_in6, and the change meant that they silently ended
up with an incorrect offset.  Changing the struct member names
triggers a compilation failure, hopefully leading to a complete fix.

2017-09-01  Florian Weimer  <fweimer@redhat.com>

	* bits/socket.h (struct sockaddr_storage): Rename __ss_padding to
	__ss_padding_reserved and __ss_align to __ss_align_reserved.
	* sysdeps/mach/hurd/bits/socket.h (struct sockaddr_storage): Likewise.
	* sysdeps/unix/sysv/linux/bits/socket.h
	(struct sockaddr_storage): Likewise.
	* inet/tst-sockaddr.c (do_test): Adjust.

Comments

Carlos O'Donell Sept. 1, 2017, 4:39 p.m. UTC | #1 
On 09/01/2017 11:16 AM, Florian Weimer wrote:
> In commit 3375cfafa7961c6ae0e509c31c3b3cef9ad1f03d (Make padding in
> struct sockaddr_storage explicit), the offsets of the members
> changed.  Some broken applications use these members to find
> the start of the address data in struct sockaddr_in or
> struct sockaddr_in6, and the change meant that they silently ended
> up with an incorrect offset.  Changing the struct member names
> triggers a compilation failure, hopefully leading to a complete fix.
> 
> 2017-09-01  Florian Weimer  <fweimer@redhat.com>
> 
> 	* bits/socket.h (struct sockaddr_storage): Rename __ss_padding to
> 	__ss_padding_reserved and __ss_align to __ss_align_reserved.
> 	* sysdeps/mach/hurd/bits/socket.h (struct sockaddr_storage): Likewise.
> 	* sysdeps/unix/sysv/linux/bits/socket.h
> 	(struct sockaddr_storage): Likewise.
> 	* inet/tst-sockaddr.c (do_test): Adjust.
We've been consistently using __glibc_reserved* prefix for this kind of thing,
but perhaps __glibc_reserved_ss_align and __glibc_reserved_ss_aligntype are
too long? 

I like that using __glibc_reserved_* clearly indicates who reserved it, and
that it is reserved like all other instances of __glibc_reserved_* should never
clash with other reserved named in other operating systems.

The patch looks good. I'm just bike shedding on the name.
diff mbox

Patch

diff --git a/bits/socket.h b/bits/socket.h
index b527f9c129..ad3c4c870f 100644
--- a/bits/socket.h
+++ b/bits/socket.h
@@ -164,8 +164,8 @@  struct sockaddr
 struct sockaddr_storage
   {
     __SOCKADDR_COMMON (ss_);	/* Address family, etc.  */
-    char __ss_padding[_SS_PADSIZE];
-    __ss_aligntype __ss_align;	/* Force desired alignment.  */
+    char __ss_padding_reserved[_SS_PADSIZE];
+    __ss_aligntype __ss_align_reserved; /* Force desired alignment.  */
   };
 
 
diff --git a/inet/tst-sockaddr.c b/inet/tst-sockaddr.c
index dccc3dac0c..bf585fbfa3 100644
--- a/inet/tst-sockaddr.c
+++ b/inet/tst-sockaddr.c
@@ -69,12 +69,12 @@  do_test (void)
          "struct sockaddr_storage size");
 
   /* Check for lack of holes in the struct definition.   */
-  check (offsetof (struct sockaddr_storage, __ss_padding)
+  check (offsetof (struct sockaddr_storage, __ss_padding_reserved)
          == __SOCKADDR_COMMON_SIZE,
          "implicit padding before explicit padding");
-  check (offsetof (struct sockaddr_storage, __ss_align)
+  check (offsetof (struct sockaddr_storage, __ss_align_reserved)
          == __SOCKADDR_COMMON_SIZE
-           + sizeof (((struct sockaddr_storage) {}).__ss_padding),
+           + sizeof (((struct sockaddr_storage) {}).__ss_padding_reserved),
          "implicit padding before explicit padding");
 
   /* Check for POSIX compatibility requirements between struct
diff --git a/sysdeps/mach/hurd/bits/socket.h b/sysdeps/mach/hurd/bits/socket.h
index 6eb09a0ab3..6e5bde458a 100644
--- a/sysdeps/mach/hurd/bits/socket.h
+++ b/sysdeps/mach/hurd/bits/socket.h
@@ -168,8 +168,8 @@  struct sockaddr
 struct sockaddr_storage
   {
     __SOCKADDR_COMMON (ss_);	/* Address family, etc.  */
-    char __ss_padding[_SS_PADSIZE];
-    __ss_aligntype __ss_align;	/* Force desired alignment.  */
+    char __ss_padding_reserved[_SS_PADSIZE];
+    __ss_aligntype __ss_align_reserved; /* Force desired alignment.  */
   };
 
 
diff --git a/sysdeps/unix/sysv/linux/bits/socket.h b/sysdeps/unix/sysv/linux/bits/socket.h
index ec2bf8564f..b3ee8a82a0 100644
--- a/sysdeps/unix/sysv/linux/bits/socket.h
+++ b/sysdeps/unix/sysv/linux/bits/socket.h
@@ -187,8 +187,8 @@  struct sockaddr
 struct sockaddr_storage
   {
     __SOCKADDR_COMMON (ss_);	/* Address family, etc.  */
-    char __ss_padding[_SS_PADSIZE];
-    __ss_aligntype __ss_align;	/* Force desired alignment.  */
+    char __ss_padding_reserved[_SS_PADSIZE];
+    __ss_aligntype __ss_align_reserved; /* Force desired alignment.  */
   };