You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Andrew Schwartzmeyer <an...@schwartzmeyer.com> on 2017/11/29 22:17:42 UTC
Please use `int_fd` instead of `int` for file descriptors
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
Re: Please use `int_fd` instead of `int` for file descriptors
Posted by Andrew Schwartzmeyer <an...@schwartzmeyer.com>.
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 <bm...@apache.org>
> wrote:
>
>> On Thu, Nov 30, 2017 at 11:12 PM, Andrew Schwartzmeyer <
>> andrew@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 <
>> >> andrew@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
>> >>>
>> >>>
>>
Re: Please use `int_fd` instead of `int` for file descriptors
Posted by Benjamin Bannier <be...@mesosphere.io>.
Hi,
> 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`.
I was wondering about this as well.
It seems like we already provide a pretty comprehensive set of stout
library functions to create file descriptors. As an example, I see
`net::socket`, so user code directly calling `::socket` seems not like
something we'd want and we should rather add missing functionality to
our library functions than completely avoid them. If we use wrappers it
should be trivial to catch undesirable use of unwrapped functions given
some list of wrapper functions. We have an existing ticket to create
such a check, https://issues.apache.org/jira/browse/MESOS-5105; please
feel to add interesting wrapper functions to it.
We might also want to consider making `int_ft` a tighter type so that
e.g., conversions to `int` require explicit user action. That might
throw another wrench into too careless work.
Cheers,
Benjamin
Re: Please use `int_fd` instead of `int` for file descriptors
Posted by Michael Park <mc...@gmail.com>.
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 <bm...@apache.org> wrote:
> On Thu, Nov 30, 2017 at 11:12 PM, Andrew Schwartzmeyer <
> andrew@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 <
> >> andrew@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
> >>>
> >>>
>
Re: Please use `int_fd` instead of `int` for file descriptors
Posted by Benjamin Mahler <bm...@apache.org>.
On Thu, Nov 30, 2017 at 11:12 PM, Andrew Schwartzmeyer <
andrew@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 <
>> andrew@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
>>>
>>>
Re: Please use `int_fd` instead of `int` for file descriptors
Posted by Andrew Schwartzmeyer <an...@schwartzmeyer.com>.
For Linux it is literally an `int`:
> using int_fd = int;
https://github.com/apache/mesos/blob/bf4bc6380cb99132736fbbefc85f3a7ca60b032c/3rdparty/stout/include/stout/os/int_fd.hpp#L35
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`?
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 <
> andrew@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
>>
Re: Please use `int_fd` instead of `int` for file descriptors
Posted by Benjamin Mahler <bm...@apache.org>.
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 <
andrew@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
>