https://github.com/labath commented:

I think we should slow down a bit.

For consistency, and to minimize the number of ifdefs, I believe windows and 
non-windows paths should use as similar approaches as possible. That means I 
think the end state should be that the posix path also uses a fork+exec 
approach.

For me, it also seems natural that this should be the first step. While doing 
that, we can make sure to create abstractions that can be used to implement the 
windows behavior as well. I'm imagining some sort of a function or a class, 
which takes the target binary, its arguments, and a socket, and launches that 
binary with that socket. (we'll probably also need a similar function/class on 
the other end to receive that socket).

The function doesn't have to be terribly generic, but it ideally should 
encompass all of platform specific behavior, so that the surrounding code does 
not need to use ifdefs. (But, ideally, it should at least be slightly generic, 
so that one can write a unit test for this functionality).

(BTW, I'm OOO for the rest of the week, so I won't be able to review this in 
detail. Upon cursory inspection, the general idea makes sense, but we really 
need to figure out how to encapsulate this. I'm not sure if we really need the 
WriteWithTimeout functionality. The reason we don't have it so far is that 
pipes have a buffer, and unless we're writing a huge chunk of data, the write 
will never block anyway. There's nothing wrong with it in principle though, and 
if you do want to have it, it'd be best if it comes with in its own patch and 
with a test.)

https://github.com/llvm/llvm-project/pull/101283
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to