NOTE: I haven't read the other responses to this thread, so I am going to start 
with this email and work my way through. More comments below.
> On Oct 1, 2014, at 1:44 PM, Zachary Turner <ztur...@google.com> wrote:
> 
> I'm trying to get ConnectionFileDescriptor working on Windows.  Because 
> select() has completely different semantics on Windows as on posix, it won't 
> work for this purpose, so It hink the best approach is to re-write 
> ConnectionFileDescriptor for windows.  I've already submitted a patch that 
> moves CFD to Host so that I can provide a different implementation for 
> Windows.  
> 
> However, I'm not sure if just providing a Windows implementation is 
> sufficient, due to the very nature of this class.  In short, the things you 
> can do with a generic file descriptor on Windows are very different than the 
> things you can do with a file descriptor on posix.  And unfortunately, 
> certain assumptions about what you can do with a file descriptor are baked 
> into the code that uses CFD as well as CFD itself (after all, its design 
> alone implies that all these different use cases of file descriptors are 
> interchangeable).
> 
> One of the more awkward things to deal with is that CFD provides a 
> constructor that takes a file descriptor.  Inside, it *assumes* that this fd 
> represents an actual file, but in practice we sometimes give it other things 
> to this constructor.
> 
> I've broken down all the cases of where we use this constructor as follows:
> 
> SBCommunication::AdoptFileDesriptor - passes in whatever fd the user decides 
> to give it, and treats it as a file.  

This is in the public API, so this needs to stay here for non windows builds. 
On windows we could add a new windows only API that uses a HANDLE if needed, or 
abstract this in some way through the public interface.

> 
> ScriptInterpreterPython::ExecuteOneLine - passes in the fd of a pipe, CFD 
> doesn't have any way to know it's a pipe though, and treats it as a file.

This makes generic use of the CFD that uses a FD. Since we need to interface 
with the native python layer, we will need to do whatever we need to do to give 
python a file handle it can use for in/out/err. This probably can be special 
cased for windows to make it work as needed.
> 
> GDBRemoteCommunicationServer::SetSTDIOFileDescriptor - Not sure, I think this 
> is always an actual file, but maybe not.
> 
> Process::SetSTDIOFileDescriptor - Same as before, I think this is always an 
> actual file, but maybe not.

These are all internal, so we can re-architect as needed.

> 
> I'm not really sure how to deal with this on Windows.  We need to know what 
> it *actually* is.  Furthermore, file descriptors in general don't provide the 
> necessary functionality required to be able to implement interrupting as used 
> by CFD.  For that we need the native OS type for whatever these objects are.  
> For sockets that means an object of type SOCKET.  For pipes and files it 
> means a HANDLE.  "Here's an fd, do something with it" just isn't enough of, 
> or the right kind of information to be able to do the right thing.

the lldb_private::Connection class is currently a very simple abstraction for 
reading/writing something that would go over a wire to talk to a remote debug 
client, but it doesn't provide interruption. 
> 
> AFAICT I only need to deal with the above 4 cases.  
> 
> The first case I can deal with by #ifdefing it out on Windows returning an 
> error that says "sorry, not supported on Windows".  

Or adding a new windows specific "HANDLE" variation for windows only and then 
make sure it interfaces with the internal classes (whatever we do with CFD).

> 
> For the second case, I'm not sure what to do because I don't actually 
> understand how this function works or why this level of complexity is 
> required.

Again, it will need to interface with the native python layer on windows. I 
don't know what that looks like or if they change any files around. Sometimes 
the current debugger has the IOHandler to to stdin/out/err and it needs to just 
use that, other times we are executing python code and want to catch the output 
so we can copy it into the result object in case the python code prints any 
output. So it needs to be generic enough to work. Everything right now on the 
IOHandler stack uses "FILE *". I really don't want to change that. Windows has 
an abstraction for "FILE *" that works, so I would just try to make things work 
on windows using the "FILE *", or there will be very major re-architecture 
required. 

The main reason for you wanting to change things is because there is no select 
on windows?

> 
> For the third and fourth cases, maybe I can just assume they're files?

Both of these two could just be changed to accept a lldb_private::Connection 
subclass. Easy fix.

> 
> I was thinking about splitting CFD into multiple classes, one for each type 
> of descriptor.  A FileConnection, PipeConnection, TcpSocketConnection, 
> ListeningConnection, etc.  Then, the caller would have to instantiate the 
> right kind.  This has its own set of problems, but at least seems to solve 
> the problem of requiring the creator of the CFD to specify what they're 
> actually giving us.  On posix in cases where it's user specified or don't 
> know, it seems we could just create a FileConnection and that would be 
> equivalent to the current behavior, since it seems to treat everything the 
> same anyway.

Feel free to do this on windows, but please leave the POSIX stuff alone. All 
file descriptor calls mostly just work with each other. There is no need to 
split these out as they are all related. They could be split out, but then a 
very intelligent base file descriptor class would be needed and then just the 
special cases can be broken out. I would rather not have to go to 5 different 
classes and change all of the implementations if they are all the same (open, 
close, read/write, etc).


_______________________________________________
lldb-dev mailing list
lldb-dev@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/lldb-dev

Reply via email to