mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Andrew Schwartzmeyer <and...@schwartzmeyer.com>
Subject Re: Review Request 66437: Windows: Removed `FD_CRT` from `WindowsFD` abstraction.
Date Fri, 06 Apr 2018 23:15:56 GMT


> On April 4, 2018, 2:49 p.m., Andrew Schwartzmeyer wrote:
> > 3rdparty/stout/include/stout/os/windows/close.hpp
> > Lines 38 (patched)
> > <https://reviews.apache.org/r/66437/diff/1/?file=1992284#file1992284line40>
> >
> >     Note that this will cause an SEH exception for `FsTest.Close` when run under
a debugger, because it purposefully closes a handle twice. Not sure what we can do about that
other than delete that part of the test.

So what do we want to do about the test? Delete that part of the test? Check if we're in a
debugger and skip that part of the test?


- Andrew


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


On April 6, 2018, 4:15 p.m., Andrew Schwartzmeyer wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66437/
> -----------------------------------------------------------
> 
> (Updated April 6, 2018, 4:15 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, Eric Mumau, John Kordich, Joseph Wu, and Michael
Park.
> 
> 
> Bugs: MESOS-8675 and MESOS-8683
>     https://issues.apache.org/jira/browse/MESOS-8675
>     https://issues.apache.org/jira/browse/MESOS-8683
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> After all the CRT APIs were replaced with Windows APIs, we no longer
> needed to support the semantics of an `int` file descriptor in
> general (in the sense of opening a CRT handle that's associated with
> the actual kernel object for the given `HANDLE`). There are specific
> use cases (usually third-party code) which still require a CRT
> int-like file descriptor, which the `crt()` function explicitly
> allocates (this allocation used to be done in the constructor).
> 
> Thus the entire `FD_CRT` type was removed from the `WindowsFD`
> abstraction. It still acts like an `int` in the sense that it can be
> constructed from one and compared to one. However, construction via
> `int` only supports the standard file descriptors 0, 1, and 2 for
> `stdin`, `stdout`, and `stderr`. Any other construction creates an
> `int_fd` which holds an `INVALID_HANDLE` value. When being compared to
> an `int`, the abstraction simply returns -1 if it is invalid (based on
> the result of the `is_valid()` method) or 0 if it is valid. This is to
> support the semantics of checking validity by something like `if (fd <
> 0)` or `if (fd == -1)`.
> 
> With the deletion of the `FD_CRT` type from `WindowsFD`, all the Stout
> APIs that switched on the type were simplified, with the last of the
> CRT code deleted.
> 
> Thanks to the introduction of the private `int get_valid()` function,
> and the removal of the `FD_CRT` type, the comparison operators became
> much simpler.
> 
> 
> Diffs
> -----
> 
>   3rdparty/stout/include/stout/os/windows/close.hpp ff635e44235d63888a210cd68d49f6678a851e31

>   3rdparty/stout/include/stout/os/windows/dup.hpp 265046cf7ffc14f7326711d295aa7dd4f0a8a1e3

>   3rdparty/stout/include/stout/os/windows/fcntl.hpp bf8c38acad60f9b0eb752053dcd53a9fda7b8bfa

>   3rdparty/stout/include/stout/os/windows/fd.hpp d7f8cdf1ad877eb55589bf5a9e75d295f91990a7

>   3rdparty/stout/include/stout/os/windows/read.hpp 8047ad590fcc46d3ec46b551472d8c518ae49cc1

>   3rdparty/stout/include/stout/os/windows/write.hpp 71006489918d9495d37d2fdfdca08b40b419481a

>   3rdparty/stout/tests/os/filesystem_tests.cpp c190baa2230298e428d4034b90dccffb59b4e710

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


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