> On March 16, 2016, 12:29 a.m., Neil Conway wrote: > > 3rdparty/libprocess/3rdparty/stout/tests/os/rmdir_tests.cpp, line 222 > > <https://reviews.apache.org/r/44874/diff/1/?file=1300429#file1300429line222> > > > > Why is this a `string&` and `newDirectory` is a `string`? I'd think > > both should probably be `string`. > > Jie Yu wrote: > Good catch, we should use 'string'. We avoid using const ref to capture > temp variable. > > Jojy Varghese wrote: > A pattern I copied from `os_tests.cpp`. Copy paste horror. I think we > need to fix it there as well as we fix it here.
Using a const ref should be safe (it _must_ be a const ref, in which case C++ will extend the lifetime of the temporary to that of the const reference -- see https://stackoverflow.com/questions/2822243/store-return-value-of-function-in-reference-c). However, it would be good to be consistent. Given RVO, I'm not sure there's anything to be gained by using the reference form. - Neil ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44874/#review123803 ----------------------------------------------------------- On March 15, 2016, 11:47 p.m., Jojy Varghese wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/44874/ > ----------------------------------------------------------- > > (Updated March 15, 2016, 11:47 p.m.) > > > Review request for mesos, Jie Yu and Neil Conway. > > > Repository: mesos > > > Description > ------- > > Added support for FTS_SLNONE in rmdir. > > > Diffs > ----- > > 3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/rmdir.hpp > cf21e7fe458626c7533e596997cab3afdabb55f4 > 3rdparty/libprocess/3rdparty/stout/tests/os/rmdir_tests.cpp > 291a22b5aae53b0bc32ae18b9343ceb5a638b37b > > Diff: https://reviews.apache.org/r/44874/diff/ > > > Testing > ------- > > make check. > > > Thanks, > > Jojy Varghese > >
