> On Feb. 16, 2014, 10:32 p.m., Jie Yu wrote: > > src/slave/containerizer/isolators/cgroups/cpushare.cpp, line 388 > > <https://reviews.apache.org/r/18175/diff/1/?file=486925#file486925line388> > > > > Unfortunately, this looks a little weird as a result of returning const > > T&. > > > > I am thinking of providing a 'const' version of operator [] in hashmap: > > > > const value_type& operator [] (const key_type& key) const; > > > > And this function internally provides a CHECK to make sure that the key > > exists. > > > > However, I am still a little hesitate because the [] semantics for > > const and non-const versions are different if we provide the overload, > > which could be quite misleading to the user. > > Ben Mahler wrote: > Will cleanup this code per benh's suggestion below.
if you want to be future-proof, rename hashmap::get to hashmap::at to match the c++11 std::map API. - Dominic ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/18175/#review34619 ----------------------------------------------------------- On Feb. 16, 2014, 3:38 p.m., Ben Mahler wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/18175/ > ----------------------------------------------------------- > > (Updated Feb. 16, 2014, 3:38 p.m.) > > > Review request for mesos, Benjamin Hindman, Jie Yu, Niklas Nielsen, and Vinod > Kone. > > > Bugs: MESOS-1008 > https://issues.apache.org/jira/browse/MESOS-1008 > > > Repository: mesos-git > > > Description > ------- > > See: https://reviews.apache.org/r/18173/ > > I also killed instances of our old pattern: > > const Try<T>& t = foo(); > > // Replaced with: > Try<T> t = foo(); > > I did this for two reasons: > 1. gcc is often already eliding the copy. > 2. This will be obviated when we add move constructors to Try, since foo() > is returning an rvalue. > > > Diffs > ----- > > src/log/replica.cpp 746d6c35c9255775ab6e70b0daf1dcecf63c16a0 > src/master/contender.cpp e3b0737195aba42c805a05c96b054cec1471b502 > src/messages/messages.hpp 98038c017a7d02c7c4f04f91c89ca56f566f707b > src/slave/containerizer/isolators/cgroups/cpushare.cpp > 989d3848343d4255c25796aa5be81dbb93a29b6e > src/slave/containerizer/isolators/cgroups/mem.cpp > a01e114c168b820e6c54af9257716b753940d510 > src/slave/slave.cpp 8ad955a71957421ae51e1becc4b06a4b6b091eb2 > src/slave/state.cpp 9af6c5b0582e972523028d226703070293b92f8b > src/tests/cgroups_tests.cpp f0dead7c09fe3cb4355c5b3b4603a2cb8c2cd3d5 > src/tests/mesos.cpp 98333a7d3a2c7c8004aacc43dda3add31d5b5487 > src/tests/values_tests.cpp c72dbc2ec05e12541c03196577dff3561bff08ed > src/tests/zookeeper_url_tests.cpp b143c539530e3319f603d9276ebf84dfecfcf6f2 > > Diff: https://reviews.apache.org/r/18175/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Ben Mahler > >
