You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Vinod Kone <vi...@gmail.com> on 2013/01/16 00:23:24 UTC

Re: Review Request: Fixed protobuf read / write calls to be POSIX compliant (handle 0, EINTR, and partial reads / writes).

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



third_party/libprocess/include/stout/protobuf.hpp
<https://reviews.apache.org/r/8695/#comment33023>

    How about
    
    Try<Nothing>::error("Failed to write protobuf size to disk: " + result.error()) ?



third_party/libprocess/include/stout/protobuf.hpp
<https://reviews.apache.org/r/8695/#comment33024>

    not yours, but can you format the params as the new style?



third_party/libprocess/include/stout/protobuf.hpp
<https://reviews.apache.org/r/8695/#comment33025>

    not yours, but can you format this, one param per line.



third_party/libprocess/include/stout/protobuf.hpp
<https://reviews.apache.org/r/8695/#comment33030>

    Can you remind me why this is Return<bool> instead of Return<Nothing> again?



third_party/libprocess/include/stout/protobuf.hpp
<https://reviews.apache.org/r/8695/#comment33026>

    Doesn't read right. Can you rephrase?
    
    How about?
    
    Instead of specifically checking for corruption, we simply try to read size bytes. If we hit EOF early, it is an indication of corruption.



third_party/libprocess/include/stout/protobuf.hpp
<https://reviews.apache.org/r/8695/#comment33029>

    How about
    
    Failed to read protobuf of size + "stringify(size) " + "bytes, because we hit  EOF unexpectedly. Possible corruption!"



third_party/libprocess/include/stout/protobuf.hpp
<https://reviews.apache.org/r/8695/#comment33031>

    not yours, but can you format the params



third_party/libprocess/include/stout/protobuf.hpp
<https://reviews.apache.org/r/8695/#comment33032>

    ditto


- Vinod Kone


On Dec. 20, 2012, 8:19 p.m., Ben Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8695/
> -----------------------------------------------------------
> 
> (Updated Dec. 20, 2012, 8:19 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Vinod Kone.
> 
> 
> Description
> -------
> 
> Please review this first: https://reviews.apache.org/r/8694/
> 
> All is well in the world again.
> 
> 
> This addresses bug MESOS-319.
>     https://issues.apache.org/jira/browse/MESOS-319
> 
> 
> Diffs
> -----
> 
>   src/slave/state.cpp 4d4c2470c44fe630ec0694ee937131b4f0aafc4e 
>   src/tests/protobuf_io_tests.cpp 979efac0f28c3d361f5c347f15933d89d9356bc9 
>   third_party/libprocess/include/stout/protobuf.hpp d2b5daedb878fcf04bc38ba912ffc0ca9a930729 
> 
> Diff: https://reviews.apache.org/r/8695/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>


Re: Review Request: Fixed protobuf read / write calls to be POSIX compliant (handle 0, EINTR, and partial reads / writes).

Posted by Ben Mahler <be...@gmail.com>.

> On Jan. 15, 2013, 11:23 p.m., Vinod Kone wrote:
> > third_party/libprocess/include/stout/protobuf.hpp, line 85
> > <https://reviews.apache.org/r/8695/diff/1/?file=241574#file241574line85>
> >
> >     Can you remind me why this is Return<bool> instead of Return<Nothing> again?

Dropped, the next in this chain does the refactor: https://reviews.apache.org/r/8696


- Ben


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


On Dec. 20, 2012, 8:19 p.m., Ben Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8695/
> -----------------------------------------------------------
> 
> (Updated Dec. 20, 2012, 8:19 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Vinod Kone.
> 
> 
> Description
> -------
> 
> Please review this first: https://reviews.apache.org/r/8694/
> 
> All is well in the world again.
> 
> 
> This addresses bug MESOS-319.
>     https://issues.apache.org/jira/browse/MESOS-319
> 
> 
> Diffs
> -----
> 
>   src/slave/state.cpp 4d4c2470c44fe630ec0694ee937131b4f0aafc4e 
>   src/tests/protobuf_io_tests.cpp 979efac0f28c3d361f5c347f15933d89d9356bc9 
>   third_party/libprocess/include/stout/protobuf.hpp d2b5daedb878fcf04bc38ba912ffc0ca9a930729 
> 
> Diff: https://reviews.apache.org/r/8695/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>