Unfortunately the root of our problem is that the cross-platform review
bots are still not up-to-snuff. Most people (including myself) won't
wait around for a bot that takes sometimes 24 hours to review the patch
after an update. So, while increasing the discoverability of `int_fd` I
think will help, _really_ we need patches to be shown as broken on
Windows (or elsewhere) before they're committed.
Off the top of my head, we've fixed a Windows build break due to `int`
vs `int_fd` at least four times, which is why I decided to send out a
heads-up email. But ideally the we hunker down and figure out how to
make the bots post build results much more quickly. I think it needs to
be under an hour for people to start paying attention to them.
Yes, we have code using int_fd within hashmaps, maps, etc, already
across
platforms so I assume it has the properties I listed on windows, but it
would be good to document that as being something that int_fd is
guaranteed
to provide.
I agree, I'll get this documented. I think this probably extends to some
other cross-platform abstractions too, I'll do a pass and see what I can
do.
On 11/30/2017 5:47 pm, Michael Park wrote:
I agree it would be nice to document
the supported operations on an `int_fd`.
I'm not sure how to actually help with
the issue of making `int_fd` more
discoverable. The only idea I've got is
a ClangTidy check to complain about
variables of type `int` named `fd` and
other similar common names for file
descriptors such as `socket`.
Thanks,
MPark
On Thu, Nov 30, 2017 at 4:41 PM Benjamin Mahler <bmah...@apache.org>
wrote:
On Thu, Nov 30, 2017 at 11:12 PM, Andrew Schwartzmeyer <
and...@schwartzmeyer.com> wrote:
> For Linux it is literally an `int`:
>
> using int_fd = int;
>>
>
> https://github.com/apache/mesos/blob/bf4bc6380cb99132736fbbe
> fc85f3a7ca60b032c/3rdparty/stout/include/stout/os/int_fd.hpp#L35
>
>
Yes I realize that :)
> So it's a safe drop-in replacement on non-Windows platforms, with all the
> same properties of an `int`.
>
> It's only on Windows where it is instead `os::WindowsFD`, and then you
may
> need to worry about its properties and semantics. Do you want these
> documented in `stout/os/windows/fd.hpp`?
Yes, we have code using int_fd within hashmaps, maps, etc, already
across
platforms so I assume it has the properties I listed on windows, but
it
would be good to document that as being something that int_fd is
guaranteed
to provide.
>
>
> On 11/30/2017 3:05 pm, Benjamin Mahler wrote:
>
>> Is it possible to document in that header the properties of int_fd that
we
>> can rely on?
>>
>> For example, it has a hash defined for use in unordered map, set, etc.
>> It's
>> a POD type, etc.
>>
>> On Wed, Nov 29, 2017 at 10:17 PM, Andrew Schwartzmeyer <
>> and...@schwartzmeyer.com> wrote:
>>
>> Hello everyone!
>>>
>>> I've realized that a lot of developers working in libprocess (and
>>> elsewhere) may not know about how to handle file descriptors in a
>>> cross-platform way for Mesos.
>>>
>>> IMPORTANT: You cannot just use `int`. File descriptors on Windows are
>>> various types of handles, but not just an `int`.
>>>
>>> The abstraction we use in Mesos is `int_fd`, found here:
>>> https://github.com/apache/mesos/blob/master/3rdparty/stout/
>>> include/stout/os/int_fd.hpp
>>>
>>> On non-Windows platforms, it's just an `int`. But on Windows, it's a
>>> `WindowsFD` which can be an `int` (from the Windows CRT which we're
>>> deprecating), a `HANDLE` (the Windows 32 API), and a `SOCKET` (from the
>>> WinSock library). If you're curious, the implementation is here:
>>> https://github.com/apache/mesos/blob/master/3rdparty/stout/
>>> include/stout/os/windows/fd.hpp
>>>
>>> I just want you to be aware that if you're writing code and need an
`int`
>>> file descriptor, please use `int_fd` (and include the appropriate
header)
>>> instead of `int`, as otherwise you break the Windows build.
>>>
>>> Thank you,
>>>
>>> Andy
>>>
>>>
