[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-devel
Subject:    Re: DNS based service discovery
From:       Thiago Macieira <thiago.macieira () kdemail ! net>
Date:       2004-10-27 19:55:20
Message-ID: 200410271655.28228.thiago.macieira () kdemail ! net
[Download RAW message or body]

[Attachment #2 (multipart/signed)]


Jakub Stachowski wrote:
>Hello,
>
>For the last month I have been working on DNS based service discovery
>(ZeroConf, Rendezvous) for KDE. You can get it from kdenonbeta/kdnssd.

Problems with the code, as it stands now:
- it's licensed GPL, so it can't be included in kdelibs. It has to be LGPL 
to be in kdelibs -- except for the ioslave and kcm.
- classes have no d-pointers or virtual hooks
- classes have inlined methods and member variables -- might not be a good 
idea
- use of C-style casts. Please replace with static_cast<> or 
reinterpret_cast<> where appropriate.

Also, please check that your conversion to QStrings in 
responder.cpp:resolve_reply are using the correct encoding (it should 
probably be UTF-8). In the same code, are you sure the 255- and 256-byte 
buffers will never overflow?

Please don't define AllServices (servicebrowser.h) as a static const QString 
variable. Add it to a class and move the definition to the .cpp file, or 
make it global and move to the .cpp (don't forget the 'extern').

In ServiceBrowser::startBrowse(), can you cache the result of 
m_domains->domains() and its end iterator? Same thing for 
UDNSQuery::havePtr().

Actually, for UDNSQuery, I'd rather you did not use QDns at all. Please use 
KResolver for PTR-resolution, if that's what you need. At the very least, 
in havePtr(), cache the QStringList and use iterators, instead of 
QStringList::at(), which can be O(n).

ServiceList is descended from QPtrList. Please make it a QValueList, since 
the QPtr* are going away in Qt4 anyways.

This is a personal taste, but don't you use indentation or break long lines? 
Your code is somewhat hard to read...

Aside from that, libhowl seem to be yet another great example of code that 
should have been written in C++.

>- good and simple API for service publishing and discovery

Seems to be achieving that. However, is it really necessary to make the 
resolutions go "_http._tcp"?

I'd suggest using two parameters and automatically prepending the 
underscore.

>- compatibility with other implementations - MacOSX, GNOME 2.8, Firefox
> plugin (under development). This is why I choose write module for mDNS
> support instead of SLP (and trying to revive knot) - because it seems to
> be quite widespread not only in operating systems and desktop
> environments but also in hardware like network printers and some wireless
> routers.

By using SRV-based resolution? I'd agree.

>- support for unicast DNS - for discovering services in larger networks
> and over internet

I also want to provide SRV-based resolution for normal stuff, including web 
browsing.

>- ioslave - for easy use from file selector, konqueror and sidebar. For
> now it can find ftp and http servers.

Probably integration with network:/ (lisa) is desireable.

>- my goal is to have kdnssd included in KDE - what do you think about it?
>"useful enough to be bugfixed and included" , " try again in several
> months" or "it sucks, just drop it" ? And if answer is "include", do you
> think it can be done in KDE 3.4?

Aside from what I pointed out above, I think we could give it a try. 
However, as things go, you'll first need an application using it for it to 
move away from kdenonbeta. Then you'll need a second for it to go into 
kdelibs.

Also, can it work at all without libhowl? How difficult would it be to 
reimplement it, if necessary?

>- I consider good API as priority ( and would hate to lower high KDE
>standards) - what should be improved?

Mentioned above.

>- there are already bits of SLP support (in krdc and kinetd) - should it
> be left as is, replaced with ZeroConf or both should be supported (for
> browsing, I suppose one standard has to be chosen for service publishing)

I can't answer that.

>There are still 3 significant improvements to be done (+ testing/bugfixing
> of course):
>
>- IPv6 support - with unicast DNS it should be no problem, but I have yet
> no idea how to do it in libhowl (form multicast DNS)

I have not seen how libhowl works, but IPv6 multicasting is fairly simple -- 
much simpler than multicasting over IPv4.

>- possibility to choose one network interface for service publishing - but
>KNetworkInterface is still placeholder and awaits choosing real
>implementation

And I don't plan on doing that for KDE 3.4. 

It seems we can rely on getifaddrs() and our replacements in order to detect 
network interfaces. And it also seems to be relatively simple to implement 
it.

However, the part relating to KMulticastSockets isn't that simple -- given 
the complexity of supporting IPv4 and IPv6 multicasting at the same time.

In any event, I did not plan to introduce new functionality in that part of 
the code for KDE 3.4, keeping it in perfect sync with the 3.3.x updates. 
That is especially because I don't have enough time to work on this till at 
least the end of the year.

>Requirements:
>- libhowl 0.9.6 . Unfortunately it crashes, patch was sent to author (also
>available in kdnssd/patches). He responded that new version of libhowl is
>going to be released in a week.
>- Qt 3.3 - there is bug in QDns  TXT record handling - it was reported and
>will be fixed in next version. Patch is also available. Good news is that
>this bug in not too serious and it probably can be worked around.

That's two requirements I would rather not have (an extra lib required in 
kdelibs and QDns).

-- 
  Thiago Macieira  -  Registered Linux user #65028
   thiago (AT) macieira (DOT) info
    ICQ UIN: 1967141   PGP/GPG: 0x6EF45358; fingerprint:
    E067 918B B660 DBD1 105C  966C 33F5 F005 6EF4 5358

[Attachment #5 (application/pgp-signature)]

>> Visit http://mail.kde.org/mailman/listinfo/kde-devel#unsub to unsubscribe <<


[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic