----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56474/#review165304 -----------------------------------------------------------
3rdparty/libprocess/src/authenticator_manager.cpp (lines 77 - 81) <https://reviews.apache.org/r/56474/#comment237161> I'm my C++ knowledge is correct, you don't need this block, doing: ```c++ authenticators_[realm].push_back(authenticator); ``` should be enough (from the [docs](http://en.cppreference.com/w/cpp/container/unordered_map/operator_at): _"operator[] is non-const because it inserts the key if it doesn't exist. If this behavior is undesirable or if the container is const, at() may be used."_) 3rdparty/libprocess/src/authenticator_manager.cpp (lines 102 - 128) <https://reviews.apache.org/r/56474/#comment237162> Not so sure about all the continues and the handling of different cases here. How about doing something like first processing the `unauthorized` items and then the `forbidden` ones, i.e. ```c++ foreach (const AuthenticationResult& result, results) { if (result.unauthorized.isNone()) { continue; } // ... Code to merge unauthorized. } if (unauthorized.isSome()) { return unauthorized; } foreach (const AuthenticationResult& result, results) { if (result.forbidden.isNone()) { continue; } // ... Code to merge forbidden. } if (forbidden.isSome()) { return forbidden; } return None(); ``` 3rdparty/libprocess/src/authenticator_manager.cpp (lines 158 - 160) <https://reviews.apache.org/r/56474/#comment237163> I think the keyword `auto` is prefered when extracting iterators, [reference](https://github.com/apache/mesos/blob/master/docs/c%2B%2B-style-guide.md#c11) - Alexander Rojas On Feb. 11, 2017, 1:44 a.m., Greg Mann wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/56474/ > ----------------------------------------------------------- > > (Updated Feb. 11, 2017, 1:44 a.m.) > > > Review request for mesos, Alexander Rojas, Jan Schlicht, Till Toenshoff, and > Vinod Kone. > > > Bugs: MESOS-7004 > https://issues.apache.org/jira/browse/MESOS-7004 > > > Repository: mesos > > > Description > ------- > > This patch updates the `AuthenticatorManager` to allow > multiple authenticators to be set for a single realm. > > > Diffs > ----- > > 3rdparty/libprocess/src/authenticator_manager.cpp > a22acd026a001788dc39b8005a56577e33c6800b > > Diff: https://reviews.apache.org/r/56474/diff/ > > > Testing > ------- > > Testing information can be found in the subsequent patch in this chain. > > > Thanks, > > Greg Mann > >
