On 10/20/06, Erik Huelsmann e.huelsmann@gmx.net wrote:
Hi Nick,
Thanks for the - now correct - patch. I'd love to react to it, but you sent both patches only to me. Was that accidental? If so, I'd like to reply to this message on-list giving my comments for consideration and discussion.
Sorry! That was, indeed, accidental.
For the benefit of those on the list:
On 10/16/06, Nick Thomas jesuswaffle@gmail.com wrote:
A couple weeks ago, I hacked together the beginnings of a TCP server socket implementation. It has documentation, but no tests. I wrote the backend for SBCL, and it seems to be working fine.
It doesn't support a lot of the requirements [that Erik] discussed. It doesn't support setting socket options, and it only works with IPv4. However, it might be useful as a basis for further work. Patch attached.
On 10/16/06, Nick Thomas jesuswaffle@gmail.com wrote:
A couple weeks ago, I hacked together the beginnings of a TCP server socket implementation. It has documentation, but no tests. I wrote the backend for SBCL, and it seems to be working fine.
It doesn't support a lot of the requirements [that Erik] discussed. It doesn't support setting socket options, and it only works with IPv4. However, it might be useful as a basis for further work. Patch attached.
Thanks! ;-)
I've copied parts of the patch into this mail below and have given comments inline.
[From README]
+ - usocket-server (class)
You decided to create a class without inheriting from the existing usocket class. I'd like to inherit from some superclass, so that we can at least use get-local-name for both server sockets and stream sockets: when a server is created with a :port 0 (any port), it may be usefull to be able to query the assigned port number afterwards...
Obviously, we can't inherit every behaviour, because there's no remote address which can be queried. We'd have to find a way to work around that. Maybe by creating a new superclass (bound-usocket or something like it) which only has a local address.
+ - server-listen (function) [ to create a passive/server socket ] + server-listen port &optional (host *default-host*) + (backlog *default-backlog-size*) + where `host' is a vectorized ip + or a string representation of a dotted ip address + or a hostname for lookup in the DNS system + `host' and `backlog' are ignored on systems that do not support + specifying them + - server-accept (method) + server-accept server-socket + returns a newly connected usocket
That all looks good.
+ - server-close (method) + server-close server-socket
well, we could inherit this one too, if we were to create the right inheritance structure.
+ - *default-host* (variable) + the default host to bind server sockets to. 0.0.0.0 by default. + - *default-backlog-size* (variable) + the default connection backlog size. 16 by default.
Ah, but supposedly, some systems only support 5 as the default value. Next to that, there's 8 in the actual definition :-)
I like how your patch makes the server-accept function return a stream related usocket.
bye,
Erik.
On 10/20/06, Erik Huelsmann ehuels@gmail.com wrote:
On 10/16/06, Nick Thomas jesuswaffle@gmail.com wrote:
A couple weeks ago, I hacked together the beginnings of a TCP server socket implementation. It has documentation, but no tests. I wrote the backend for SBCL, and it seems to be working fine.
It doesn't support a lot of the requirements [that Erik] discussed. It doesn't support setting socket options, and it only works with IPv4. However, it might be useful as a basis for further work. Patch attached.
Thanks! ;-)
I've copied parts of the patch into this mail below and have given comments inline.
[From README]
- usocket-server (class)
You decided to create a class without inheriting from the existing usocket class. I'd like to inherit from some superclass, so that we can at least use get-local-name for both server sockets and stream sockets: when a server is created with a :port 0 (any port), it may be usefull to be able to query the assigned port number afterwards...
Obviously, we can't inherit every behaviour, because there's no remote address which can be queried. We'd have to find a way to work around that. Maybe by creating a new superclass (bound-usocket or something like it) which only has a local address.
I did consider inheriting from USOCKET, but I decided not to, because of the problems you mentioned. However, I don't have any objection to making a common superclass for both types of socket. Actually, it would be a win in a couple different ways: it would make the API more symmetrical, we could share some code between the two socket types in some backends, and it would give us room for extensions when we want to add UDP sockets.
One question that comes to mind is: should the slot containing the implementation-specific socket object be in the common superclasses or in the subclasses? This is not immediately obvious because some implementations use distinct types for peer and server sockets, while others use the same type.
In my opinion, the slot should be in the common superclass, because, in the case of implementations which have a unified socket type, we can define the common functions (GET-LOCAL-* and SOCKET-CLOSE) as a single method over the common superclass, and, in the case of implementations with separate socket types, we can define two methods over the subclasses.
- server-close (method)
server-close server-socketwell, we could inherit this one too, if we were to create the right inheritance structure.
Yes, we could, as described above. I don't really see any reason not to do so.
- *default-host* (variable)
the default host to bind server sockets to. 0.0.0.0 by default.
- *default-backlog-size* (variable)
the default connection backlog size. 16 by default.Ah, but supposedly, some systems only support 5 as the default value.
I didn't know that bit of trivia. Might as well change it, then.
Next to that, there's 8 in the actual definition :-)
Oops. :)
I like how your patch makes the server-accept function return a stream related usocket.
Thanks!
Here's a patch (based on svn) which implements some of the modifications discussed. Specifically, it fixes *DEFAULT-BACKLOG-SIZE* and factors out the common parts of USOCKET and USOCKET-SERVER to a base class, USOCKET-BASIC, and makes various API changes (none which break backcompat with 0.1.0, of course) to accomodate the new scheme. Also, for symmetry, I rename USOCKET to USOCKET-PEER, with USOCKET as a deprecated alias.
On 10/20/06, nick thomas jesuswaffle@gmail.com wrote:
On 10/20/06, Erik Huelsmann ehuels@gmail.com wrote:
On 10/16/06, Nick Thomas jesuswaffle@gmail.com wrote:
A couple weeks ago, I hacked together the beginnings of a TCP server socket implementation. It has documentation, but no tests. I wrote the backend for SBCL, and it seems to be working fine.
It doesn't support a lot of the requirements [that Erik] discussed. It doesn't support setting socket options, and it only works with IPv4. However, it might be useful as a basis for further work. Patch attached.
Thanks! ;-)
I've copied parts of the patch into this mail below and have given comments inline.
[From README]
- usocket-server (class)
You decided to create a class without inheriting from the existing usocket class. I'd like to inherit from some superclass, so that we can at least use get-local-name for both server sockets and stream sockets: when a server is created with a :port 0 (any port), it may be usefull to be able to query the assigned port number afterwards...
Obviously, we can't inherit every behaviour, because there's no remote address which can be queried. We'd have to find a way to work around that. Maybe by creating a new superclass (bound-usocket or something like it) which only has a local address.
I did consider inheriting from USOCKET, but I decided not to, because of the problems you mentioned. However, I don't have any objection to making a common superclass for both types of socket. Actually, it would be a win in a couple different ways: it would make the API more symmetrical, we could share some code between the two socket types in some backends, and it would give us room for extensions when we want to add UDP sockets.
One question that comes to mind is: should the slot containing the implementation-specific socket object be in the common superclasses or in the subclasses? This is not immediately obvious because some implementations use distinct types for peer and server sockets, while others use the same type.
In my opinion, the slot should be in the common superclass, because, in the case of implementations which have a unified socket type, we can define the common functions (GET-LOCAL-* and SOCKET-CLOSE) as a single method over the common superclass, and, in the case of implementations with separate socket types, we can define two methods over the subclasses.
- server-close (method)
server-close server-socketwell, we could inherit this one too, if we were to create the right inheritance structure.
Yes, we could, as described above. I don't really see any reason not to do so.
- *default-host* (variable)
the default host to bind server sockets to. 0.0.0.0 by default.
- *default-backlog-size* (variable)
the default connection backlog size. 16 by default.Ah, but supposedly, some systems only support 5 as the default value.
I didn't know that bit of trivia. Might as well change it, then.
Next to that, there's 8 in the actual definition :-)
Oops. :)
I like how your patch makes the server-accept function return a stream related usocket.
Thanks!
On 10/21/06, nick thomas jesuswaffle@gmail.com wrote:
Here's a patch (based on svn) which implements some of the modifications discussed. Specifically, it fixes *DEFAULT-BACKLOG-SIZE* and factors out the common parts of USOCKET and USOCKET-SERVER to a base class, USOCKET-BASIC, and makes various API changes (none which break backcompat with 0.1.0, of course) to accomodate the new scheme. Also, for symmetry, I rename USOCKET to USOCKET-PEER, with USOCKET as a deprecated alias.
Nice. I started on implementing this too, but then decided I needed to understand the problem domain better, so I wrote down (some) of what I have in my head regarding the final class structure.
Unfortunately, I didn't get much further than this:
Class structure ===============
usocket | +- datagram-usocket +- stream-usocket - stream-server-usocket
The usocket class will have methods to query local properties, such as:
- get-local-name: to query to which interface the socket is bound - <other socket and protocol options such as SO_REUSEADDRESS>
In my implementation, I renamed usocket to stream-usocket. Then I moved the socket slot from stream-usocket to the new usocket superclass.
If all internal consumers are adjusted to create stream-usockets, then that should be backward compatible too, iff external libraries don't directly create usocket instances, that is....
Because we're pre-1.0, I don't want to introduce too much compatibility cruft yet. How about adding a section 'Pre-1.0 Interface guarantees'? I'd like to guarantee socket-connect for the total life-time pre-1.0 and I'd like to guarantee that on the socket class it creates, you can call the different get-* methods, as well as use the 'socket' and 'stream' accessors. Shouldn't that be enough to guarantee 'normal' use? If you agree with that, I think we can rename usocket-basic -> usocket and usocket-peer -> stream-usocket. Apart from that, the patch should be good. (I can do the renames locally, no need for resubmission!)
Your patch includes #FIXME comments about :external-format. I had some discussions about that on IRC and will send a separate mail on the subject.
Thanks for thinking with me!
bye,
Erik.
-
Erik Huelsmann -
nick thomas