mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Joseph Wu <jos...@mesosphere.io>
Subject Re: Review Request 66439: Windows: Made `protobuf::write()` use CRT file descriptor explicitly.
Date Thu, 19 Apr 2018 21:07:45 GMT

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




3rdparty/stout/include/stout/protobuf.hpp
Line 83 (original), 87-95 (patched)
<https://reviews.apache.org/r/66439/#comment282859>

    I'm a bit surprised this doesn't cause problems with tests like `ProtobufIOTest.Basic`
and `OperationStatusUpdateManagerTests`.
    
    Since you're dup-ing the FDs for each write, if the caller passes in the same FD for multiple
calls, the original FD will retain its offset (near the beginning of the file) and should
repeatedly overwrite the data each time `protobuf::write` is called.


- Joseph Wu


On April 6, 2018, 4:16 p.m., Andrew Schwartzmeyer wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66439/
> -----------------------------------------------------------
> 
> (Updated April 6, 2018, 4:16 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, Eric Mumau, John Kordich, Joseph Wu, and Michael
Park.
> 
> 
> Bugs: MESOS-8675
>     https://issues.apache.org/jira/browse/MESOS-8675
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This is another edge case where a third-party library (protobuf)
> requires a CRT integer file descriptor. Thus we duplicate the `int_fd`
> and then explicitly allocate via `crt()`, which requires that we also
> manually close it via `_close()`.
> 
> 
> Diffs
> -----
> 
>   3rdparty/stout/include/stout/protobuf.hpp 2fa5072e3c62c487da0dccffdd38d2fa1a615dc0

> 
> 
> Diff: https://reviews.apache.org/r/66439/diff/2/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>


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