[Lldb-commits] [PATCH] Apply SOCK_CLOEXEC flag to socket API functions in order to avoid handle leakage to a forked child process.

[Zachary Turner]

Sounds good to me.

On Tue Nov 11 2014 at 11:30:31 AM Greg Clayton <clayborg at gmail.com> wrote:

>
> > On Nov 10, 2014, at 8:03 PM, Oleksiy Vyalov <ovyalov at google.com> wrote:
> >
> >>> ! In D6204#10, @zturner wrote:
> >> Can I also request that the variable *not* be called close_on_exec?.
> It is
> >> possible to implement an equivalent on Windows, but calling it close on
> >> exec will be confusing to people on Windows.  Can we come up with a more
> >> generic name?  On Windows this would be called "inherit handles", which
> to
> >> me sounds like a better name than close on exec, but I may be biased.
> Is
> >> "inherit handles" confusing to people on non-Windows?
> >>
> >> In other words, I'm proposing
> >>
> >> virtual lldb::ConnectionStatus ConnectionFileDescriptor::Connect(const
> char
> >> *s, Error *error_ptr, bool close_on_exec = true);
> >>
> >> would become
> >>
> >> virtual lldb::ConnectionStatus ConnectionFileDescriptor::Connect(const
> char
> >> *s, Error *error_ptr, bool inherit_handles = false);
> >
> > It sounds good to me to call this variable inherit_handles in order to
> make it more platform-independent.
>
> Handles is a windows term and doesn't make sense for unix. How about
> "child_processes_inherit"?
>
> > As I can see ConnectionFileDescriptor::Connect is overrides pure method
> from Connection::Connect (const char *url, Error *error_ptr)  and I'm a bit
> reluctant to make such variable visible for Communication layer  - so, I'm
> thinking about extending  ConnectionFileDescriptor constructor in a way
> like this:
> >
> > ConnectionFileDescriptor(int fd, bool owns_fd, bool inherit_handles =
> false);
> >
> > In this case methods within ConnectionFileDescriptor::Connect
> (SocketListen, NamedSocketAccept, NamedSocketAccept, ConnectTCP,
> ConnectUDP) may use member field m_inherit_handles instead of taking
> additional argument.
> >
> > http://reviews.llvm.org/D6204
> >
> >
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20141111/60fb4863/attachment.html>