----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/15112/#review27860 -----------------------------------------------------------
Ship it! 3rdparty/libprocess/include/process/shared.hpp <https://reviews.apache.org/r/15112/#comment54258> Not sure I'd expose this one, do we need it yet? 3rdparty/libprocess/include/process/shared.hpp <https://reviews.apache.org/r/15112/#comment54257> Not sure I'd expose this one, do we need it yet? 3rdparty/libprocess/include/process/shared.hpp <https://reviews.apache.org/r/15112/#comment54251> : data(new Data(CHECK_NOTNULL(t))) {} Also, are these the same semantics as shared_ptr? I.e., can you not instantiate a shared_ptr with NULL to start? 3rdparty/libprocess/include/process/shared.hpp <https://reviews.apache.org/r/15112/#comment54252> How about two checks? CHECK(data); CHECK_NOTNULL(data->t); I think the implicit bool operator is a bit weird though and I'd almost prefer: CHECK_NOTNULL(data.get()); CHECK_NOTNULL(data->t); Thoughts? 3rdparty/libprocess/include/process/shared.hpp <https://reviews.apache.org/r/15112/#comment54253> And 't' can be NULL here right? 3rdparty/libprocess/include/process/shared.hpp <https://reviews.apache.org/r/15112/#comment54254> Same thoughts here as above re: implicit bool operator. 3rdparty/libprocess/include/process/shared.hpp <https://reviews.apache.org/r/15112/#comment54255> s/upgrade attempt is detected/upgrade is already being performed/ 3rdparty/libprocess/include/process/shared.hpp <https://reviews.apache.org/r/15112/#comment54256> Nice! 3rdparty/libprocess/include/process/shared.hpp <https://reviews.apache.org/r/15112/#comment54259> Do we need any memory fencing here? 3rdparty/libprocess/src/tests/shared_tests.cpp <https://reviews.apache.org/r/15112/#comment54266> I think we should treat a Shared<T> just like we do a T* and since we don't do 'if (t) {' I don't think we should support the bool operator on Shared right now but instead require 'if (t.get() != NULL) {'. I think being explicit here is better. - Benjamin Hindman On Oct. 31, 2013, 5:05 a.m., Jie Yu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/15112/ > ----------------------------------------------------------- > > (Updated Oct. 31, 2013, 5:05 a.m.) > > > Review request for mesos, Benjamin Hindman, Ben Mahler, Vinod Kone, and Jiang > Yan Xu. > > > Repository: mesos-git > > > Description > ------- > > See summary. > > > Diffs > ----- > > 3rdparty/libprocess/include/process/shared.hpp faa11cc > 3rdparty/libprocess/src/tests/shared_tests.cpp e520b4f > > Diff: https://reviews.apache.org/r/15112/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Jie Yu > >
