----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60139/#review178098 -----------------------------------------------------------
I do not agree with this change. An `Owned` cannot be copied semantically. If we pass an `Owned` at all, it should be by value, not by reference, so potential copies happen on interface boundaries and we do not need to perform potentially unneeded copies inside the consuming functions. See e.g., https://herbsutter.com/2013/06/05/gotw-91-solution-smart-pointer-parameters/ for a more detailed write-up. I believe the existing code is still broken since it performs copies of an `Owned` (which is currently allowed syntactically, https://issues.apache.org/jira/browse/MESOS-5122). IMHO the correct fix would be to keep the existing signatures, but to `std::move` when passing the `authenticator` value on, both when used as function argument and when ultimatly stored in `AuthenticatorManagerProcess::authenticators_`. This would both prevent copies and express ownership semantics correctly. - Benjamin Bannier On June 16, 2017, 1:16 a.m., Neil Conway wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/60139/ > ----------------------------------------------------------- > > (Updated June 16, 2017, 1:16 a.m.) > > > Review request for mesos and Alexander Rojas. > > > Repository: mesos > > > Description > ------- > > Spotted via clang-tidy. > > > Diffs > ----- > > 3rdparty/libprocess/src/authenticator_manager.hpp > 0dc8fd24b411d649bcc62208bde5784cac4ea997 > 3rdparty/libprocess/src/authenticator_manager.cpp > 5cbed53e7085f227d90679e1b56ad803d9b74a47 > > > Diff: https://reviews.apache.org/r/60139/diff/1/ > > > Testing > ------- > > `make check` > > > Thanks, > > Neil Conway > >
