Add create windows socket functionality. #24
Conversation
This enables our app to act as a named pipe server, which is useful when implementing remote drivers on Windows. Signed-off-by: Michal Kostrzewa <kostrzewa.michal@o2.pl>
| ) | ||
|
|
||
| // NewWindowsSocket creates a Windows named pipe on the specified path. | ||
| func NewWindowsSocket(addr string, pipeConfig *winio.PipeConfig) (net.Listener, error) { |
There was a problem hiding this comment.
Follow the conventions of Go's net package. This should be called Listen.
What is the purpose of adding this function? It looks like it could just be this:
var ListenWindowsSocket = winio.ListenPipe
There was a problem hiding this comment.
I tried to make this symmetrical to unix_socket.go and tcp_socket.go. Should I rename and refactor "NewUnixSocket" and "NewTCPSocket" for this PR?
There was a problem hiding this comment.
This package is an unmitigated disaster.
I guess just leave it as is.
Honestly, I don't see what value this function provides. It seems to be a passthrough.
There was a problem hiding this comment.
It pretty much is... Again, I added it for symmetry with other New*Socket functions. I'm not fond of the design either but I tried to adhere to it.
I can swap it with var ListenWindowsSocket = winio.ListenPipe if you think it'd be better.
I hear that you want to get rid of go-connections and I won't try to stop you. In any case, this changeset is mostly required for my other PR that depends on it docker/go-plugins-helpers#63
There was a problem hiding this comment.
I moved Listener creation to go-plugins-helpers PR, this PR can be closed.
|
@m-kostrzewa Thank you for understanding. I'd rather not have this in here, especially if is just a pass through. |
This enables our app to act as a named pipe listener, which is useful when implementing remote drivers on Windows.
Signed-off-by: Michal Kostrzewa kostrzewa.michal@o2.pl