KExtendedSocket oddities

[Thiago Macieira]

Malte Starostik wrote:
>* KSocketAddress inherits QObject, defines Q_OBJECT but doesn't have any
>  signals nor slots nor does it use any QObject functionality AFAICS. This
>  seems unneccessarily inefficient (in my situation I need to keep some
>  hundred addresses). Also it makes KSocketAddress noncopyable, IMHO it
> would be nice to have a copyable, value-based class instead. Of course this
> would totally break BC

These were my first KDE classes, so I didn't know better at the time. And, 
come to think of it, I had never noticed so.

>* KExtendedSocket of course inherits QObject...but: its ctors don't take a
>  QObject* parent argument, so one cannot use QObject's nice auto-deletion
>  feature :-(
>  Adding overloads with such a parameter wouldn't break anything, would it?

Probably not a problem. Again, it's the same problem as above: my first 
classes, so I didn't know better...

>* setBindXXX() doesn't work on passive sockets. This is clearly documented,
>  yet it looks unnatural to me. In most cases when you bind a socket, it's a
>  passive one. Took me a while to find out that a passive socket is bound to
>  the address set with setHost() etc. instead. IMHO they should either be
>  equivalent for passive sockets or exactly the other way round as now.

Suggestion noted. IMO, setBindXXX() and setXXX() should then instead do the 
same job.

Maybe for KDE4 we could rename the setXXX functions to setRemoteXXX... I'm 
open for suggestions.

>* setAddressReusable() does nothing when called before listen().
>  But that means bind() is called before one gets a chance to set
>  SO_REUSEADDR. Not sure if this is a problem, but I've always seen and
>  used SO_REUSEADDR _before_ calling bind().

Hmm... noted. Shouldn't be hard to fix it. The reason it works that way is 
quite simple: there is no socket before KExtendedSocket::listen() is called.

>* service and host resolution works primarily on QString. The overloads that
>  take a number for the port argument use QString::number() and then call
> the QString versions. Why is this? Isn't a simple htons() far cheaper than
> int => QString => resolution? :-) Same for IP addresses, okay here I can
> see this would get difficult wrt IPv6 support. Still, what about overloads
> that take an in_addr/in6_addr and skip any host resolution?

Services and hosts should be considered strings, not numbers. Don't ask me to 
change that. That's exactly the reason I've called them "services", not 
ports. If you use a number, you lose all possibility of SRV-based name lookup 
in the future.

However, I didn't know at the time and I still don't know if a QString or a 
QCString should be used. I've chosen to use a QString thinking that i18n'ed 
hostnames could come in the future and KExtendedSocket could handle them 
nicely, without having to change all the upstream code, in order to handle 
international hostnames. I think that, even today, the encoding to be used 
for international hostnames is not yet defined...

As for simple IP addresses and ports, which I imagine comes from a name 
resolution or a secure source (e.g., user input), yes, we could use numbers. 
But the underlying resolving function (getaddrinfo) takes char* parameters, 
so we'd end up converting back to strings. The reason we use getaddrinfo() is 
exactly the point of not having and not wanting to handle exotic sockets 
types. We let the underlying system work with them and just give us back nice 
replies, of which we don't have to know a thing about. 

So, instead, you can use KSocketAddress's nodeName() and serviceName() with 
noResolve. What I can, however, add is a method that takes an entire 
KSocketAddress or an entire KAddressInfo. Remember that socket addresses are 
more than just hostnames and ports.

>* It's possible to create datagram sockets, which is nice. But somehow
>  KExtendedSocket lacks wrappers around sendto() and recvfrom() to
>  actually use such a socket, getting info about a packet's source and all.

Yes, it does... I'll get around to adding those.

>* One last minor thing: the enumerator names like streamSocket,
> passiveSocket begin with a lowercase letter which contradicts most if not
> all other enums in kdelibs.

Sorry, didn't know better at the time...

Anyways, I should say that I've been planning a big rewrite in that code for 
KDE4. The idea that was proposed to me was to dump the existing KSocket and 
KServerSocket and, instead, make them be derived from QSocket and use the KDE 
functionality. That would mean adding a KSocketDevice to be derived from 
QSocketDevice. And as a part of that rewrite, it could be that 
KExtendedSocket disappears and gets instead replaced by those other classes 
-- or not. I've been meaning to post a message to ask for input on that, and 
now it seems a good time as ever.

-- 
  Thiago Macieira - UFOT Registry number: 1001
 thiagom at mail.com
   ICQ UIN: 1967141  PGP/GPG: 0x6EF45358
     Registered Linux user #65028
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: signature
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20021213/f6e311c4/attachment.sig>