mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Joris Van Remoortere <joris.van.remoort...@gmail.com>
Subject Re: Review Request 49424: Added an abstraction os::raw::Argv in stout.
Date Sat, 02 Jul 2016 17:11:32 GMT

-----------------------------------------------------------
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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message