[boost] Re: Re: Sockets!

From: Jessie Hernandez (jessie_at_[hidden])
Date: 2003-10-27 15:22:44

"Rob Lievaart" <Rob.Lievaart_at_[hidden]> wrote in message
news:29D7A7147DC1D611919C0008028AC184018B4E73_at_post.int.nob.nl...

[snip]

> - 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
>    this:
>
>       typedef ipv4_domain domain;
>       domain::address_type addr( host<domain>( "www.boost.org"
> ).address().ip(), 80 );
>       connector<domain> conn;
>       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,
> but
>    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)
>

Thanks, I'll look into it.

>    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
> existence
>    of the different domains.  Probably only the address_type needs to be
> aware
>    that differences exist, but it could hide it from most of the library
>    and from the user.
>

Well, I provided the domain template parameter everywhere because not all
socket types may use the same system functions. For example, local domain
sockets exist on Unix, but not on Windows, but these can be easily faked on
Windows using file operations (this emulation is on my TODO list). It would
not be possible to emulate local domain sockets on Windows if the socket
classes could not be specialized for local_domain.

> -  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.)
>

I agree with you also, but I saw the Connector pattern mentioned in several
places (including the Boost socket Wiki), so I just provided it for that
reason. If someone could enlighten us to why it is better to include a
separate Connector class instead of just adding a connect() method to the
socket class, that will be greatly appreciated.

> -  Exceptions:
>    You have a large exception hierarchy.  There has been some debate on
the
> use
>    of large exception hierarchies versus a single exception with error
> codes.
>    I am not sure if there was a winner, but personnally I would prefer a
> single
>    socket exception with errorcodes.
>    You currently have 20 or so exception classes and they are members of
> either
>    the socket_base or the host_base _class_ instead of boost::net or where
> ever
>    the library would live.
>

Well, I personally prefer a "large" socket exception hierarchy. This makes
it very easy to catch specific exceptions and provides better error
messages. With the current STL file classes, you may get errors such as
"ios_base::failbit set!" (since there is only one exception class,
ios_base::failure). This doesn't tell me why the failure happened (does the
user have permissions to the file, etc.?) I'd rather have classes such as
socket_base::connection_refused, etc., which have their what() methods
overloaded to print out more specific messages.

The reason I have the exceptions be part of the classes is because: 1) the
exceptions belong only at the class which logically owns them (e.g., there
is a boost::net::host::not_found exception and a
boost::net::protocol::not_found exception), and 2) it follows existing
standard (at least with ios_base::failure).

I know the classes can be easily renamed to boost::net::host_not_found and
boost::net::protocol_not_found, but what benefits would this provide? If
everyone would prefer for the exceptions to be at the boost::net:: level,
I'll gladly do it.

> -  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?
>

Good point. Originally I thought of using domain_base polymorphically, but
later abandoned the idea. I'll remove domain_base and address_base shortly.

> -  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.
>

Well, there may be two implementations available in the system (normal
socket API and WinSock on Windows, for example).
If it isn't a member, then a base class can be used, as you said. But then
the implementation needs to be a template parameter in the basic_socket
class to accomodate normal sockets and WinSock sockets. I'll also look into
this.

> 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:
>
>     http://oakroadsystems.com/tech/cppredef.htm
>

Yes, I finished making this change yesterday (I originally did this so it
could match the additional implementation definitions seen in popular STL
implementations, but recently changed it after seeing a post regarding
this). I'll upload it as soon as I finish the design document.

> -  I see quite a few ifdefs related to the socklen_t type,
>    e.g.:
>
>     #if defined( WIN32 )
>         int _NameLength = sizeof( address_type::native_t );
>     #else
>         socklen_t _NameLength = sizeof( address_type::native_t );
>     #endif
>
>    These could be replaced by a single typedef somewhere.  Makes
>    the code a lot simpler.
>

I only saw two of these, but you have a valid point. I'll also do this in
one typedef.

> -  The shutdown function taking an ios_base::openmode as parameter
>    seems strange.
>

Well, I just did it for reuse of ios_base::in and ios_base::out, but I can
easily have an openmode enum in socket_base.

> -  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
> wraps
>    a socket, so maybe rdbuf should be enough here, from there you can
>    go to the socket, iff required.
>

Will do.

>
> Just my $0.02,
>

Thanks for your comments!

--
Jessie Hernandez