Re: Review Request 33274: Fix capture by reference of temporary strings in Libprocess.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33274/ --- (Updated June 2, 2015, 12:37 p.m.) Review request for mesos, Bernd Mathiske, Cody Maloney, Joerg Schad, Michael Park, and Till Toenshoff. Changes --- rebased. Bugs: MESOS-2631 https://issues.apache.org/jira/browse/MESOS-2631 Repository: mesos Description --- See summary. Diffs (updated) - 3rdparty/libprocess/src/help.cpp ce2de11370f890e2f6eccd357839f8841fcc3395 3rdparty/libprocess/src/process.cpp e3de3cd6b536aaaf59784360aed546512dd04dc9 3rdparty/libprocess/src/tests/decoder_tests.cpp efe364a5de38c4d39fe42df243df173e7d04479a 3rdparty/libprocess/src/tests/encoder_tests.cpp 784a2c71a2a7d349726f8e85bab3888b2c405aba 3rdparty/libprocess/src/tests/http_tests.cpp d29cd29d8c0544671a09d204ca8ba4f24340e2de 3rdparty/libprocess/src/tests/process_tests.cpp 67e582cc250a9767a389e2bd0cc68985477f3ffb 3rdparty/libprocess/src/tests/subprocess_tests.cpp dea4ed8699c6c7cf5b4f5d3f014577c474dcac95 Diff: https://reviews.apache.org/r/33274/diff/ Testing --- make check Thanks, Joris Van Remoortere
Re: Review Request 33274: Fix capture by reference of temporary strings in Libprocess.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33274/#review86191 --- Ship it! Ship It! - Benjamin Hindman On May 20, 2015, 10:36 p.m., Joris Van Remoortere wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/33274/ > --- > > (Updated May 20, 2015, 10:36 p.m.) > > > Review request for mesos, Bernd Mathiske, Cody Maloney, Joerg Schad, Michael > Park, and Till Toenshoff. > > > Bugs: MESOS-2631 > https://issues.apache.org/jira/browse/MESOS-2631 > > > Repository: mesos > > > Description > --- > > See summary. > > > Diffs > - > > 3rdparty/libprocess/src/help.cpp 85e1bdec8d7e8f46477d0f3d88847baeca2dcc9c > 3rdparty/libprocess/src/process.cpp > e3de3cd6b536aaaf59784360aed546512dd04dc9 > 3rdparty/libprocess/src/tests/decoder_tests.cpp > efe364a5de38c4d39fe42df243df173e7d04479a > 3rdparty/libprocess/src/tests/encoder_tests.cpp > 784a2c71a2a7d349726f8e85bab3888b2c405aba > 3rdparty/libprocess/src/tests/http_tests.cpp > d29cd29d8c0544671a09d204ca8ba4f24340e2de > 3rdparty/libprocess/src/tests/process_tests.cpp > 67e582cc250a9767a389e2bd0cc68985477f3ffb > 3rdparty/libprocess/src/tests/subprocess_tests.cpp > dea4ed8699c6c7cf5b4f5d3f014577c474dcac95 > > Diff: https://reviews.apache.org/r/33274/diff/ > > > Testing > --- > > make check > > > Thanks, > > Joris Van Remoortere > >
Re: Review Request 33274: Fix capture by reference of temporary strings in Libprocess.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33274/#review84621 --- Ship it! Ship It! - Michael Park On May 20, 2015, 10:36 p.m., Joris Van Remoortere wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/33274/ > --- > > (Updated May 20, 2015, 10:36 p.m.) > > > Review request for mesos, Bernd Mathiske, Cody Maloney, Joerg Schad, Michael > Park, and Till Toenshoff. > > > Bugs: MESOS-2631 > https://issues.apache.org/jira/browse/MESOS-2631 > > > Repository: mesos > > > Description > --- > > See summary. > > > Diffs > - > > 3rdparty/libprocess/src/help.cpp 85e1bdec8d7e8f46477d0f3d88847baeca2dcc9c > 3rdparty/libprocess/src/process.cpp > e3de3cd6b536aaaf59784360aed546512dd04dc9 > 3rdparty/libprocess/src/tests/decoder_tests.cpp > efe364a5de38c4d39fe42df243df173e7d04479a > 3rdparty/libprocess/src/tests/encoder_tests.cpp > 784a2c71a2a7d349726f8e85bab3888b2c405aba > 3rdparty/libprocess/src/tests/http_tests.cpp > d29cd29d8c0544671a09d204ca8ba4f24340e2de > 3rdparty/libprocess/src/tests/process_tests.cpp > 67e582cc250a9767a389e2bd0cc68985477f3ffb > 3rdparty/libprocess/src/tests/subprocess_tests.cpp > dea4ed8699c6c7cf5b4f5d3f014577c474dcac95 > > Diff: https://reviews.apache.org/r/33274/diff/ > > > Testing > --- > > make check > > > Thanks, > > Joris Van Remoortere > >
Re: Review Request 33274: Fix capture by reference of temporary strings in Libprocess.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33274/ --- (Updated May 20, 2015, 10:36 p.m.) Review request for mesos, Bernd Mathiske, Cody Maloney, Joerg Schad, Michael Park, and Till Toenshoff. Changes --- rebased. fixed missing case. Bugs: MESOS-2631 https://issues.apache.org/jira/browse/MESOS-2631 Repository: mesos Description --- See summary. Diffs (updated) - 3rdparty/libprocess/src/help.cpp 85e1bdec8d7e8f46477d0f3d88847baeca2dcc9c 3rdparty/libprocess/src/process.cpp e3de3cd6b536aaaf59784360aed546512dd04dc9 3rdparty/libprocess/src/tests/decoder_tests.cpp efe364a5de38c4d39fe42df243df173e7d04479a 3rdparty/libprocess/src/tests/encoder_tests.cpp 784a2c71a2a7d349726f8e85bab3888b2c405aba 3rdparty/libprocess/src/tests/http_tests.cpp d29cd29d8c0544671a09d204ca8ba4f24340e2de 3rdparty/libprocess/src/tests/process_tests.cpp 67e582cc250a9767a389e2bd0cc68985477f3ffb 3rdparty/libprocess/src/tests/subprocess_tests.cpp dea4ed8699c6c7cf5b4f5d3f014577c474dcac95 Diff: https://reviews.apache.org/r/33274/diff/ Testing --- make check Thanks, Joris Van Remoortere
Re: Review Request 33274: Fix capture by reference of temporary strings in Libprocess.
> On May 12, 2015, 11:28 p.m., Michael Park wrote: > > Looks good overall! > > > > I think found one more: > > > > `src/process.cpp` > > ```cpp > > 2717: const string& name = tokens.size() > 1 ? tokens[1] : ""; > > ``` I didn't even know you could capture by reference in this case! - Alexander --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33274/#review83488 --- On April 22, 2015, 8:11 p.m., Joris Van Remoortere wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/33274/ > --- > > (Updated April 22, 2015, 8:11 p.m.) > > > Review request for mesos, Bernd Mathiske, Cody Maloney, Joerg Schad, Michael > Park, and Till Toenshoff. > > > Bugs: MESOS-2631 > https://issues.apache.org/jira/browse/MESOS-2631 > > > Repository: mesos > > > Description > --- > > See summary. > > > Diffs > - > > 3rdparty/libprocess/src/help.cpp 85e1bdec8d7e8f46477d0f3d88847baeca2dcc9c > 3rdparty/libprocess/src/process.cpp > 97ac09fd10b767bc46387abc3657d005a00830c8 > 3rdparty/libprocess/src/tests/decoder_tests.cpp > efe364a5de38c4d39fe42df243df173e7d04479a > 3rdparty/libprocess/src/tests/encoder_tests.cpp > 784a2c71a2a7d349726f8e85bab3888b2c405aba > 3rdparty/libprocess/src/tests/http_tests.cpp > dfdb233de93552ab7040647ee1958a85d47c9482 > 3rdparty/libprocess/src/tests/process_tests.cpp > eb38edce2c483ba7f963a826893a15a075238618 > 3rdparty/libprocess/src/tests/subprocess_tests.cpp > dea4ed8699c6c7cf5b4f5d3f014577c474dcac95 > > Diff: https://reviews.apache.org/r/33274/diff/ > > > Testing > --- > > make check > > > Thanks, > > Joris Van Remoortere > >
Re: Review Request 33274: Fix capture by reference of temporary strings in Libprocess.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33274/#review83488 --- Looks good overall! I think found one more: `src/process.cpp` ```cpp 2717: const string& name = tokens.size() > 1 ? tokens[1] : ""; ``` - Michael Park On April 22, 2015, 6:11 p.m., Joris Van Remoortere wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/33274/ > --- > > (Updated April 22, 2015, 6:11 p.m.) > > > Review request for mesos, Bernd Mathiske, Cody Maloney, Joerg Schad, Michael > Park, and Till Toenshoff. > > > Bugs: MESOS-2631 > https://issues.apache.org/jira/browse/MESOS-2631 > > > Repository: mesos > > > Description > --- > > See summary. > > > Diffs > - > > 3rdparty/libprocess/src/help.cpp 85e1bdec8d7e8f46477d0f3d88847baeca2dcc9c > 3rdparty/libprocess/src/process.cpp > 97ac09fd10b767bc46387abc3657d005a00830c8 > 3rdparty/libprocess/src/tests/decoder_tests.cpp > efe364a5de38c4d39fe42df243df173e7d04479a > 3rdparty/libprocess/src/tests/encoder_tests.cpp > 784a2c71a2a7d349726f8e85bab3888b2c405aba > 3rdparty/libprocess/src/tests/http_tests.cpp > dfdb233de93552ab7040647ee1958a85d47c9482 > 3rdparty/libprocess/src/tests/process_tests.cpp > eb38edce2c483ba7f963a826893a15a075238618 > 3rdparty/libprocess/src/tests/subprocess_tests.cpp > dea4ed8699c6c7cf5b4f5d3f014577c474dcac95 > > Diff: https://reviews.apache.org/r/33274/diff/ > > > Testing > --- > > make check > > > Thanks, > > Joris Van Remoortere > >
Re: Review Request 33274: Fix capture by reference of temporary strings in Libprocess.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33274/#review82875 --- Ship it! Agree with Adam's benchmark comment, would be great. - Joerg Schad On April 22, 2015, 6:11 p.m., Joris Van Remoortere wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/33274/ > --- > > (Updated April 22, 2015, 6:11 p.m.) > > > Review request for mesos, Bernd Mathiske, Cody Maloney, Joerg Schad, Michael > Park, and Till Toenshoff. > > > Bugs: MESOS-2631 > https://issues.apache.org/jira/browse/MESOS-2631 > > > Repository: mesos > > > Description > --- > > See summary. > > > Diffs > - > > 3rdparty/libprocess/src/help.cpp 85e1bdec8d7e8f46477d0f3d88847baeca2dcc9c > 3rdparty/libprocess/src/process.cpp > 97ac09fd10b767bc46387abc3657d005a00830c8 > 3rdparty/libprocess/src/tests/decoder_tests.cpp > efe364a5de38c4d39fe42df243df173e7d04479a > 3rdparty/libprocess/src/tests/encoder_tests.cpp > 784a2c71a2a7d349726f8e85bab3888b2c405aba > 3rdparty/libprocess/src/tests/http_tests.cpp > dfdb233de93552ab7040647ee1958a85d47c9482 > 3rdparty/libprocess/src/tests/process_tests.cpp > eb38edce2c483ba7f963a826893a15a075238618 > 3rdparty/libprocess/src/tests/subprocess_tests.cpp > dea4ed8699c6c7cf5b4f5d3f014577c474dcac95 > > Diff: https://reviews.apache.org/r/33274/diff/ > > > Testing > --- > > make check > > > Thanks, > > Joris Van Remoortere > >
Re: Review Request 33274: Fix capture by reference of temporary strings in Libprocess.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33274/#review81267 --- Is it possible to do some quick benchmarking to show that this change doesn't compile into more copies, making Mesos run slower and fatter? - Adam B On April 22, 2015, 11:11 a.m., Joris Van Remoortere wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/33274/ > --- > > (Updated April 22, 2015, 11:11 a.m.) > > > Review request for mesos, Bernd Mathiske, Cody Maloney, Joerg Schad, Michael > Park, and Till Toenshoff. > > > Bugs: MESOS-2631 > https://issues.apache.org/jira/browse/MESOS-2631 > > > Repository: mesos > > > Description > --- > > See summary. > > > Diffs > - > > 3rdparty/libprocess/src/help.cpp 85e1bdec8d7e8f46477d0f3d88847baeca2dcc9c > 3rdparty/libprocess/src/process.cpp > 97ac09fd10b767bc46387abc3657d005a00830c8 > 3rdparty/libprocess/src/tests/decoder_tests.cpp > efe364a5de38c4d39fe42df243df173e7d04479a > 3rdparty/libprocess/src/tests/encoder_tests.cpp > 784a2c71a2a7d349726f8e85bab3888b2c405aba > 3rdparty/libprocess/src/tests/http_tests.cpp > dfdb233de93552ab7040647ee1958a85d47c9482 > 3rdparty/libprocess/src/tests/process_tests.cpp > eb38edce2c483ba7f963a826893a15a075238618 > 3rdparty/libprocess/src/tests/subprocess_tests.cpp > dea4ed8699c6c7cf5b4f5d3f014577c474dcac95 > > Diff: https://reviews.apache.org/r/33274/diff/ > > > Testing > --- > > make check > > > Thanks, > > Joris Van Remoortere > >
Re: Review Request 33274: Fix capture by reference of temporary strings in Libprocess.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33274/ --- (Updated April 22, 2015, 6:11 p.m.) Review request for mesos, Bernd Mathiske, Cody Maloney, Joerg Schad, Michael Park, and Till Toenshoff. Bugs: MESOS-2631 https://issues.apache.org/jira/browse/MESOS-2631 Repository: mesos Description --- See summary. Diffs - 3rdparty/libprocess/src/help.cpp 85e1bdec8d7e8f46477d0f3d88847baeca2dcc9c 3rdparty/libprocess/src/process.cpp 97ac09fd10b767bc46387abc3657d005a00830c8 3rdparty/libprocess/src/tests/decoder_tests.cpp efe364a5de38c4d39fe42df243df173e7d04479a 3rdparty/libprocess/src/tests/encoder_tests.cpp 784a2c71a2a7d349726f8e85bab3888b2c405aba 3rdparty/libprocess/src/tests/http_tests.cpp dfdb233de93552ab7040647ee1958a85d47c9482 3rdparty/libprocess/src/tests/process_tests.cpp eb38edce2c483ba7f963a826893a15a075238618 3rdparty/libprocess/src/tests/subprocess_tests.cpp dea4ed8699c6c7cf5b4f5d3f014577c474dcac95 Diff: https://reviews.apache.org/r/33274/diff/ Testing --- make check Thanks, Joris Van Remoortere
