-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/49424/#review140451
-----------------------------------------------------------




3rdparty/stout/include/stout/os/raw/argv.hpp (line 27)
<https://reviews.apache.org/r/49424/#comment205837>

    I know you want to say `NULL` to be explicit here. Consider using `nullptr` 
to avoid tripping people grepping for `NULL`.



3rdparty/stout/include/stout/os/raw/argv.hpp (line 40)
<https://reviews.apache.org/r/49424/#comment205896>

    It seems like this extra vector and the subsequent copy into the argv array 
is only necessary because you're supporting structures with an unknown size.
    
    Would this be simpler if you restricted to inputs that support `.size()`?



3rdparty/stout/include/stout/os/raw/argv.hpp (line 43)
<https://reviews.apache.org/r/49424/#comment205897>

    Wouldn't hurt to have a small comment here mentioning that `strcpy` does 
the null termination.


- Joris Van Remoortere


On June 30, 2016, 4:18 p.m., Jie Yu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49424/
> -----------------------------------------------------------
> 
> (Updated June 30, 2016, 4:18 p.m.)
> 
> 
> Review request for mesos and Joris Van Remoortere.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added an abstraction os::raw::Argv in stout.
> 
> 
> Diffs
> -----
> 
>   3rdparty/stout/include/Makefile.am f10c836c1ac008cc4055741648b5e7dd697e4c1e 
>   3rdparty/stout/include/stout/os.hpp 
> 53b00932693fba7cf6514da6a519269a904de345 
>   3rdparty/stout/include/stout/os/raw/argv.hpp PRE-CREATION 
>   3rdparty/stout/tests/os_tests.cpp 786da14addf62be5f5270f156fb1e011d3f403e3 
> 
> Diff: https://reviews.apache.org/r/49424/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Reply via email to