[phobos] Breaking changes for std.socket improvement

[Andrei Alexandrescu]

I already wrote the following code review for 
http://bitbucket.org/repeatedly/scrap/src/tip/socket.d before seeing 
Masahiro's new message that we should scrap that review. I'm sending 
this in case it applies to his new work based on Asio.

* Line 67: don't use typedef - it will go away

* 87: feel free to insert a static assert(sockaddr_storage.sizeof == 
...) to make sure the compiler did as you expected.

* 270: enforce(val, new SocketException(...))

* 304-309: lowercase? We have visibility protection because of 
AddressFamily.

* 362: Why is Protocol a class? It has all public members and no 
overridable methods. (Also, constructors and public members often are 
questionable.)

* 433: Same question about Service.

* 539: And same question about InternetHost.

* 546: why not a ref hostent?

* 553: no need for the .idup

* 663: I'm weary about every module adding its own exception type, but 
this is subject to a separate investigation we should do for all of Phobos.

* 694-698: Public members of a class mean that I can change any of them 
to random values without breaking the consistency of the object. Is that 
true for AddressInfo?

* 752: ref addrinfo

* 777 and others: which of these methods do you think should be 
overridable? Probably a small subset if any. Then they should be final.

* 947: Is there something that naturally differentiates the two types of 
addresses? If so, you could get rid of Type.

* 1294: I'm not an IPv6 pro, but looks like a little hierarchy would 
work here (with e.g. IPv4 and IPv6 inheriting a common base). Maybe 
LocalAddress could be snuck in too.

* 2310: what does byes mean?

* 2520, 2541 etc.: Return size_t.

* 2934: select() has well-known issues. Any plans to support the newer 
system-dependent APIs or libevent?


Andrei

On 07/05/2010 02:08 PM, Masahiro Nakagawa wrote:
> std.socket lacks some features and I don't like current design. So, I
> rewrote this module.
>
> Summary:
>
> - Change API of Protocol, Service, and InternetHost.
>
> -----
> /* old */
> Protocol proto = new Protocol; // construct before check
> enforce(proto.getProtocolByType(ProtocolType.TCP)); // check
> // do stuff
>
> /* new */
> Protocol proto = enforce(Protocol.getByType(ProtocolType.TCP)); //
> construct after check
> // do stuff
> -----
>
> I leaves these classes for aliases(getaddrinfo can't get aliases).
>
> - Add AddressInfo class(wraps C's addrinfo struct)
>
> AddressInfo resolves some address familes. I added AF_UNIX support.
> This class is useful for Socket and Endpoint initialization.
>
> In the future, above classes should be moved to std.net(or std.net.dns).
>
> - Endpoint
>
> I think Address should not have port. Address is a address, not a endpoint.
> So I added Endpoint structs(derived their name from Asio) and Endpoints
> use corresponding Addresses.
>
> -- IPEndpoint
>
> This struct supports IPv4 and IPv6 using IPAddress.
>
> -- LocalEndpoint
>
> This struct supports local path using LocalAddress.
>
> - Socket
>
> Socket is a template bacause Socket should support any address familes.
> Old Socket uses class-inheritance, but newFamilyObject is
> bad(newFamilyObject can't use user-defined class).
>
> Some methods of old Socket used return-value checking :(
>
> - SocketSet
>
> add and remove methods are typesafe-variadic function.
>
> - Add const and attribute
>
> Sources:
>
> - socket.d
> http://bitbucket.org/repeatedly/scrap/src/tip/socket.d
>
> unittest is a simple example.
>
> - socketstream.d
> http://bitbucket.org/repeatedly/scrap/src/tip/socketstream.d
>
> std.stream will be replaced or eliminated. This module need?
>
> - LocalEndpoint sample
> http://bitbucket.org/repeatedly/scrap/src/tip/sample/echo_server.d
> http://bitbucket.org/repeatedly/scrap/src/tip/sample/echo_client.d
>
> Contribution from satoru_h
>
>
> I would like to commit this change. What do you think?
>
>
> Masahiro
> _______________________________________________
> phobos mailing list
> phobos at puremagic.com
> http://lists.puremagic.com/mailman/listinfo/phobos