From: Rob Lievaart (Rob.Lievaart_at_[hidden])
Date: 2003-10-27 04:55:32
Some quick feedback, I haven't actually tried to use it.
In general I think a socket framework would be usefull,
but as you can see in the vault a lot of people have tried,
and non has succeeded so far.
> I'd like to get feedback on the current state of the library
> and see what
> can be improved (I'm mostly interested in the design aspect).
Which is not documented, so hard to comment on. :-)
- I like the idea of having streams, that can wrap around sockets,
they can make it easy to use sockets in a lot of places.
- ipv4 vs. ipv6
From what I understand, to talk to a host I need to do something like
typedef ipv4_domain domain;
domain::address_type addr( host<domain>( "www.boost.org"
).address().ip(), 80 );
basic_socketstream<domain, char> sockstream( "stream:tcp" );
This means that my code is locked to ipv4, should boost ever become
reachable via ipv6 then I'll still use ipv4 and if that is ever
dropped I'm out of luck.
Sometimes you want to be able to specify exactly what protocol to use,
usually, I don't care. It would be nice if the library could handle this
so in the default case I don't have to think about it and my program
will work with both ipv6 and ipv4 hosts/clients. (if and when available)
(In Unix Network Programming, W. Richard Stevens describes some C wrapper
functions that lets you treat IPV4, IPV6, and Unix domain sockets, more
or less transparently)
Also I wonder if the domain really needs to be a parameter everywhere.
IIRC, the only difference at the level of the socket api is
size fo the address structure being passed around, and the handling
of gethostbyname/getaddrinfo functions. And some extra coket options.
I expect it is possible to make most of the library unaware of the
of the different domains. Probably only the address_type needs to be
that differences exist, but it could hide it from most of the library
and from the user.
- I see a connector , I assume there will also be an acceptor
to accompany the "server_socket"? I am still not to comfortable with
the connector/acceptor idea. Acceptors have to do some work, listen
on a socket, create a new one, etc. so it makes some sense to create
a seperate object for that. And then a connector looks good with that,
but a whole object just for a call to
::connect( ... )
feels kind of overdoing it to me.
(I know loads of people disagree with me, I just thought I'd mention it.)
You have a large exception hierarchy. There has been some debate on the
of large exception hierarchies versus a single exception with error
I am not sure if there was a winner, but personnally I would prefer a
socket exception with errorcodes.
You currently have 20 or so exception classes and they are members of
the socket_base or the host_base _class_ instead of boost::net or where
the library would live.
- Host class
Here you also have the domain ipv4,ipv6 in the type, but a host could
have both an ipv4 and ipv6 stack, and AFAIK most current host that have
IPV6 also have an ipv4 stack. So in this case, the address list of
a host would contain ipv4 and ipv6 addresses.
- I wonder what the use of the abstract base classes domain_base and
address_base is. It looks to me like you only use their subclasses
in templates and never plymorphically, so why would you make a
baseclass with virtual functions?
- Why the split into basic_socket and basic_socket implementation,
what is the use of that? All the members of basic_socket_impl
are virtual, but I don't see any case of wanting to replace this
with something else at runtime.
It looks to me like you are using it to share implementation between
basic_socket and server_socket, but wouldn't it be easier to
do this using a common baseclass. I could have missed it but
I see no use for the seperate implementation class.
Some implementation issues:
- You like to use underscores in variable names, names starting with
double underscores, or starting with an underscore and all caps are
reserved. They could clash with compiler defined names, so you
should avoid them. Take a look at:
- I see quite a few ifdefs related to the socklen_t type,
#if defined( WIN32 )
int _NameLength = sizeof( address_type::native_t );
socklen_t _NameLength = sizeof( address_type::native_t );
These could be replaced by a single typedef somewhere. Makes
the code a lot simpler.
- The shutdown function taking an ios_base::openmode as parameter
- It would be nice if the connector also accepted socket streams, instead
of needing to pass stream->rdsocket(). Also I am not sure if you want to
expose the socket at this level, the socket streams wrap a buffer that
a socket, so maybe rdbuf should be enough here, from there you can
go to the socket, iff required.
Just my $0.02,
Boost list run by bdawes at acm.org, gregod at cs.rpi.edu, cpdaniel at pacbell.net, john at johnmaddock.co.uk