You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Andrew Schwartzmeyer <an...@schwartzmeyer.com> on 2017/12/19 03:34:40 UTC

Review Request 64689: Windows: Fixed `os::open()` to always use `O_BINARY`.

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

Review request for mesos, Akash Gupta, Jeff Coffler, Gaston Kleiman, and Joseph Wu.


Repository: mesos


Description
-------

Previously, we had been manually adding the `O_BINARY` flag as we
encountered bugs due to the Windows default behavior of performing
line-ending translation. This was error prone.

Given this precedent, it seems safe to assume that all our existing code
expects the POSIX semantics of "binary mode", that is, no translation of
written data at all. So now we add this flag by default in `os::open()`
instead of in the users.

It is possible that a future use requires text translation. At such
point, we can trivially fix `os::open()` to take a boolean flag to
control the addition of `O_BINARY`, but we do not currently need to
engineer this.


Diffs
-----

  3rdparty/stout/include/stout/net.hpp 7b6557d40a83e1572bdc2dd89b5ff99fb8ed696a 
  3rdparty/stout/include/stout/os/open.hpp 1443b63d260b3da38073f234d15ffb4b97d4a736 
  3rdparty/stout/include/stout/os/write.hpp 4c718b1a5055a742f16cac0bc5e88aa4ab6acfcd 
  3rdparty/stout/include/stout/protobuf.hpp baad12648dd78ab72ea4277f4c7f99da16696a40 


Diff: https://reviews.apache.org/r/64689/diff/1/


Testing
-------


Thanks,

Andrew Schwartzmeyer


Re: Review Request 64689: Windows: Fixed `os::open()` to always use `O_BINARY`.

Posted by Gaston Kleiman <ga...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/64689/#review194127
-----------------------------------------------------------


Ship it!




Ship it!!! It would have saved me a lot of time last week! =).

- Gaston Kleiman


On Dec. 18, 2017, 7:34 p.m., Andrew Schwartzmeyer wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64689/
> -----------------------------------------------------------
> 
> (Updated Dec. 18, 2017, 7:34 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, Jeff Coffler, Gaston Kleiman, and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Previously, we had been manually adding the `O_BINARY` flag as we
> encountered bugs due to the Windows default behavior of performing
> line-ending translation. This was error prone.
> 
> Given this precedent, it seems safe to assume that all our existing code
> expects the POSIX semantics of "binary mode", that is, no translation of
> written data at all. So now we add this flag by default in `os::open()`
> instead of in the users.
> 
> It is possible that a future use requires text translation. At such
> point, we can trivially fix `os::open()` to take a boolean flag to
> control the addition of `O_BINARY`, but we do not currently need to
> engineer this.
> 
> 
> Diffs
> -----
> 
>   3rdparty/stout/include/stout/net.hpp 7b6557d40a83e1572bdc2dd89b5ff99fb8ed696a 
>   3rdparty/stout/include/stout/os/open.hpp 1443b63d260b3da38073f234d15ffb4b97d4a736 
>   3rdparty/stout/include/stout/os/write.hpp 4c718b1a5055a742f16cac0bc5e88aa4ab6acfcd 
>   3rdparty/stout/include/stout/protobuf.hpp baad12648dd78ab72ea4277f4c7f99da16696a40 
> 
> 
> Diff: https://reviews.apache.org/r/64689/diff/1/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>


Re: Review Request 64689: Windows: Fixed `os::open()` to always use `O_BINARY`.

Posted by Jeff Coffler <je...@taltos.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/64689/#review194167
-----------------------------------------------------------


Ship it!




Ship It!

- Jeff Coffler


On Dec. 19, 2017, 3:34 a.m., Andrew Schwartzmeyer wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64689/
> -----------------------------------------------------------
> 
> (Updated Dec. 19, 2017, 3:34 a.m.)
> 
> 
> Review request for mesos, Akash Gupta, Jeff Coffler, Gaston Kleiman, and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Previously, we had been manually adding the `O_BINARY` flag as we
> encountered bugs due to the Windows default behavior of performing
> line-ending translation. This was error prone.
> 
> Given this precedent, it seems safe to assume that all our existing code
> expects the POSIX semantics of "binary mode", that is, no translation of
> written data at all. So now we add this flag by default in `os::open()`
> instead of in the users.
> 
> It is possible that a future use requires text translation. At such
> point, we can trivially fix `os::open()` to take a boolean flag to
> control the addition of `O_BINARY`, but we do not currently need to
> engineer this.
> 
> 
> Diffs
> -----
> 
>   3rdparty/stout/include/stout/net.hpp 7b6557d40a83e1572bdc2dd89b5ff99fb8ed696a 
>   3rdparty/stout/include/stout/os/open.hpp 1443b63d260b3da38073f234d15ffb4b97d4a736 
>   3rdparty/stout/include/stout/os/write.hpp 4c718b1a5055a742f16cac0bc5e88aa4ab6acfcd 
>   3rdparty/stout/include/stout/protobuf.hpp baad12648dd78ab72ea4277f4c7f99da16696a40 
> 
> 
> Diff: https://reviews.apache.org/r/64689/diff/2/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>