Why did you guys choose to use 'auto*' here? These are the first instances in the code base, can we remove them? Let's consider 'auto&' and 'auto*' and update the style guide first, IMHO 'auto*' and 'auto&' are less intuitive, so we may want to be more conservative about them in general.
On Thu, Dec 3, 2015 at 12:39 AM, <[email protected]> wrote: > Repository: mesos > Updated Branches: > refs/heads/master 0a82c2b12 -> 49d3cb112 > > > Fixed pointer alignment error in IP::create(). > > The previous code took the address of a `struct sockaddr`, and then cast > the > resulting pointer to `struct sockaddr_storage *`. The alignment > requirements > for `struct sockaddr_storage *` are more strict than for `struct sockaddr > *`, > and hence this code produced undefined behavior per ubsan in GCC 5.2. > > Drive-by Fix: Updated the code to use `reinterpret_cast` rather than a > C-style > cast, and to preserve constness. > > Review: https://reviews.apache.org/r/40435 > > > Project: http://git-wip-us.apache.org/repos/asf/mesos/repo > Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/49d3cb11 > Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/49d3cb11 > Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/49d3cb11 > > Branch: refs/heads/master > Commit: 49d3cb1125cfcc8644e7f37b8d8654729aecd657 > Parents: 0a82c2b > Author: Neil Conway <[email protected]> > Authored: Thu Dec 3 03:13:22 2015 -0500 > Committer: Michael Park <[email protected]> > Committed: Thu Dec 3 03:21:50 2015 -0500 > > ---------------------------------------------------------------------- > .../3rdparty/stout/include/stout/ip.hpp | 39 ++++++++++++++------ > 1 file changed, 27 insertions(+), 12 deletions(-) > ---------------------------------------------------------------------- > > > > http://git-wip-us.apache.org/repos/asf/mesos/blob/49d3cb11/3rdparty/libprocess/3rdparty/stout/include/stout/ip.hpp > ---------------------------------------------------------------------- > diff --git a/3rdparty/libprocess/3rdparty/stout/include/stout/ip.hpp > b/3rdparty/libprocess/3rdparty/stout/include/stout/ip.hpp > index 3e506e1..1d34d4e 100644 > --- a/3rdparty/libprocess/3rdparty/stout/include/stout/ip.hpp > +++ b/3rdparty/libprocess/3rdparty/stout/include/stout/ip.hpp > @@ -211,23 +211,38 @@ inline Try<IP> IP::parse(const std::string& value, > int family) > > inline Try<IP> IP::create(const struct sockaddr_storage& _storage) > { > - switch (_storage.ss_family) { > - case AF_INET: { > - struct sockaddr_in addr = *((struct sockaddr_in*) &_storage); > - return IP(addr.sin_addr); > - } > - default: { > - return Error( > - "Unsupported family type: " + > - stringify(_storage.ss_family)); > - } > - } > + // According to POSIX: (IEEE Std 1003.1, 2004) > + // > + // (1) `sockaddr_storage` is "aligned at an appropriate boundary so that > + // pointers to it can be cast as pointers to protocol-specific address > + // structures and used to access the fields of those structures without > + // alignment problems." > + // > + // (2) "When a `sockaddr_storage` structure is cast as a `sockaddr` > + // structure, the `ss_family` field of the `sockaddr_storage` structure > + // shall map onto the `sa_family` field of the `sockaddr` structure." > + // > + // Therefore, casting from `const sockaddr_storage*` to `const > sockaddr*` > + // (then subsequently dereferencing the `const sockaddr*`) should be > safe. > + // Note that casting in the reverse direction (`const sockaddr*` to > + // `const sockaddr_storage*`) would NOT be safe, since the former might > + // not be aligned appropriately. > + const auto* addr = reinterpret_cast<const struct sockaddr*>(&_storage); > + return create(*addr); > } > > > inline Try<IP> IP::create(const struct sockaddr& _storage) > { > - return create(*((struct sockaddr_storage*) &_storage)); > + switch (_storage.sa_family) { > + case AF_INET: { > + const auto* addr = reinterpret_cast<const struct > sockaddr_in*>(&_storage); > + return IP(addr->sin_addr); > + } > + default: { > + return Error("Unsupported family type: " + > stringify(_storage.sa_family)); > + } > + } > } > > > >
