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

Zachary Turner zturner at google.com
Wed Nov 12 15:50:55 PST 2014


>>! In D6204#22, @ovyalov wrote:
> I agree with Zach that extending Connect() method in such way may expose file-specific details to Connection class dependencies. Additionally there are a few unclear things for me here:
> - As I can see LLVM/LLDB functions/methods follow the convention where Error is always a last parameter if provided.
> - If options struct is defined in ConnectionFileDescriptor it means that this struct type should be visible to Socket. In this case we may have mutual referencing between Socket and ConnectionFileDescriptor.
> - If default constructor for the Options sets m_child_processes_inherit to true it means that we need to pass explicitly Options{false} at each ConnectionFileDescriptor::Connect call site, right?
> 
> Thank you.

I don't think we need an options struct at all.  Why not just make it a bool?

Another, perhaps better option instead of passing it into the default constructor is to make a public method on CFD called SetChildProcessInherit(bool).  This way you can set the value, call Connect, set the value to something else, call connect again, etc.

Since this inheritance is specific to files, anyone who wants to call this method already knows they're working with a ConnectionFileDescriptor, and not some other kind of Connection, so they already have a pointer or reference to a ConnectionFileDescriptor, and not the base class.  So they can easily call this method.  Then just pass it through as a bool, and not as an Options structure.

http://reviews.llvm.org/D6204






More information about the lldb-commits mailing list