| Message ID | 20160519131238.1C38540113D03@oldenburg.str.redhat.com |
|---|---|
| State | New |
| Headers | show |
fweimer@redhat.com (Florian Weimer) writes: > diff --git a/bits/socket.h b/bits/socket.h > index ab9f242..b3f2753 100644 > --- a/bits/socket.h > +++ b/bits/socket.h > @@ -164,6 +164,7 @@ struct sockaddr > struct sockaddr_storage > { > __SOCKADDR_COMMON (ss_); /* Address family, etc. */ > + char __ss_padding_init[sizeof (__ss_aligntype) - __SOCKADDR_COMMON_SIZE]; That needs to be __alignof, otherwise you change the layout, and you need to watch out for zero padding. Andreas.
On 05/19/2016 03:40 PM, Andreas Schwab wrote: > fweimer@redhat.com (Florian Weimer) writes: > >> diff --git a/bits/socket.h b/bits/socket.h >> index ab9f242..b3f2753 100644 >> --- a/bits/socket.h >> +++ b/bits/socket.h >> @@ -164,6 +164,7 @@ struct sockaddr >> struct sockaddr_storage >> { >> __SOCKADDR_COMMON (ss_); /* Address family, etc. */ >> + char __ss_padding_init[sizeof (__ss_aligntype) - __SOCKADDR_COMMON_SIZE]; > > That needs to be __alignof, otherwise you change the layout, and you > need to watch out for zero padding. Hmm. This would have to be conditional on __GNUC__. What if we put the padding in the middle, like this? #define _SS_PADSIZE \ _SS_SIZE - __SOCKADDR_COMMON_SIZE - sizeof (__ss_aligntype) struct sockaddr_storage { __SOCKADDR_COMMON (ss_); /* Address family, etc. */ char __ss_padding[_SS_PADSIZE]; __ss_aligntype __ss_align; /* Force desired alignment. */ }; Then there should be no undeclared padding, and the __ss_align member should increase the alignment of the entire struct. Thanks, Florian
Florian Weimer <fweimer@redhat.com> writes: > What if we put the padding in the middle, like this? > > #define _SS_PADSIZE \ > _SS_SIZE - __SOCKADDR_COMMON_SIZE - sizeof (__ss_aligntype) That is different from the current definition, potentially increasing the size of the structure. The problem with the current definition is that it assumes that offsetof (struct sockaddr_storage, __ss_align) == sizeof (__ss_aligntype), which is not guaranteed. Andreas.
On 05/19/2016 05:18 PM, Andreas Schwab wrote: > Florian Weimer <fweimer@redhat.com> writes: > >> What if we put the padding in the middle, like this? >> >> #define _SS_PADSIZE \ >> _SS_SIZE - __SOCKADDR_COMMON_SIZE - sizeof (__ss_aligntype) > > That is different from the current definition, potentially increasing > the size of the structure. The problem with the current definition is > that it assumes that offsetof (struct sockaddr_storage, __ss_align) == > sizeof (__ss_aligntype), which is not guaranteed. Are you saying that before my changes, sizeof (struct sockaddr_storage) != _SS_SIZE? Which in-tree ports are affected by this? m68k perhaps, because _ss_aligntype has just two-byte alignment? (I'm not concerned about out-of-tree ports which redefine __SOCKADDR_COMMON, they can fix this up in any way they want.) Thanks, Florian
Florian Weimer <fweimer@redhat.com> writes: > Are you saying that before my changes, sizeof (struct sockaddr_storage) != > _SS_SIZE? Which in-tree ports are affected by this? m68k perhaps, > because _ss_aligntype has just two-byte alignment? Yes, it's 126 there. Andreas.
diff --git a/bits/socket.h b/bits/socket.h index ab9f242..b3f2753 100644 --- a/bits/socket.h +++ b/bits/socket.h @@ -164,6 +164,7 @@ struct sockaddr struct sockaddr_storage { __SOCKADDR_COMMON (ss_); /* Address family, etc. */ + char __ss_padding_init[sizeof (__ss_aligntype) - __SOCKADDR_COMMON_SIZE]; __ss_aligntype __ss_align; /* Force desired alignment. */ char __ss_padding[_SS_PADSIZE]; }; diff --git a/sysdeps/mach/hurd/bits/socket.h b/sysdeps/mach/hurd/bits/socket.h index 02c5dac..8d1c2fd 100644 --- a/sysdeps/mach/hurd/bits/socket.h +++ b/sysdeps/mach/hurd/bits/socket.h @@ -168,6 +168,7 @@ struct sockaddr struct sockaddr_storage { __SOCKADDR_COMMON (ss_); /* Address family, etc. */ + char __ss_padding_init[sizeof (__ss_aligntype) - __SOCKADDR_COMMON_SIZE]; __ss_aligntype __ss_align; /* Force desired alignment. */ char __ss_padding[_SS_PADSIZE]; }; diff --git a/sysdeps/unix/sysv/linux/bits/socket.h b/sysdeps/unix/sysv/linux/bits/socket.h index 0581c79..448b436 100644 --- a/sysdeps/unix/sysv/linux/bits/socket.h +++ b/sysdeps/unix/sysv/linux/bits/socket.h @@ -166,6 +166,7 @@ struct sockaddr struct sockaddr_storage { __SOCKADDR_COMMON (ss_); /* Address family, etc. */ + char __ss_padding_init[sizeof (__ss_aligntype) - __SOCKADDR_COMMON_SIZE]; __ss_aligntype __ss_align; /* Force desired alignment. */ char __ss_padding[_SS_PADSIZE]; };