Review for gkiagia's krdc port to Tp::StreamTubeClient

[Olli Salli]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

It's this one

http://cgit.collabora.com/git/user/gkiagia/krdc.git/commit/?h=new-tubes-api&id=a106f14176570982cab4f7200095764d7bdd29c2

first of all, ugh, the old code used synchronous D-Bus to e.g. accept
the tube! Good riddance.

We already went through in IRC whether some code would be needed -
answer: no - and removed it.

>     connect(m_stubeClient.data(),
> SIGNAL(tubeAcceptedAsTcp(QHostAddress,quint16,QHostAddress,quint16,

Yeah, this is all that's needed for basic operation. But I think you
should also connect to tubeClosed to clean up from your hash tubes which
WE don't close, but get closed for other reasons. E.g. server connection
reconnect, or the remote going away.

Or are you confident that closeTube will always get invoked in all those
occasions via some other mechanism? (Likely: getting disconnect event /
error on the server socket - but would that really call the functions to
INITIATE a disconnect like disconnectHost or closeTab?)

You might at this point want to investigate using the parameters for
passing some kind of credentials to avoid accidental / undesired access
to the rfb socket, as we discussed in the krfb review. Setting them when
exporting the socket in krfb, you could recover them tube->parameters()
in your onTubeAccepted() slot, then use to do rfb authentication or
whatever (as much as that's supported by the protocol libraries used).

Otherwise looks perfect - and wonderfully simple indeed, thanks to
StreamTubeClient.

I think we're now in a point where implementing tubes enabled software
is pretty much easy enough code-wise for broader adoption. However,
understanding enough of the spec to write the .client files still
remains an issue, though (not visible in this patch, as that was already
done for the old code, however). Maybe we should add some API to
StreamTubeClient/StreamTubeServer, which would write to a given
QIODevice the .client file corresponding to what services they were
created for, without even having to export with it / set it to accept
tubes? One could then write a tiny application to hook this to their
build system. Opinions?

Another option is generalized .client file writing from introspecting
running Clients, but that'd require a dbus session, potentially GUI if
the tubes code isn't separated enough to its own module etc... so I
think that's not as useful, and would be a lot more involved.

Or maybe just a simple python script, which you give --p2pservices
rfb,ftp --roomservices ftp or alike, and it then fills a template which
it prints to stdout? Simplest of them all, but unified way to create the
handler for generating the file and then for actually registering and
using it would be nice. That'd also generalize better if we add more
attributes besides the tube service name to STC/STS creation parameters
affecting the resulting filter. Dunno...

Br,
Olli
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iQEcBAEBAgAGBQJOjw+AAAoJEAQQkupGanj4P/MIALXnfHzyBdSPn+6Xy8KCF7c5
PlNRWeSr9+ERFSG5D9xSV3OTfsMxtkD/fphp5h1DDen6XhDm5lUdIUpSJpg2CaIR
fiAGrkKTIQKzBFD4vSjADEirQ9f8zg8XhLR0KkKCHmyj46jXDTTO0RRc+z4aMb3H
fZ800sDVLAj3TrPnEdll+qK27T0pdj59IqMry5eFO85TGsZha+emhRXgmMSUqjia
WkrsxcbZ+JKl1ucf73bPK5avCtX8/WOZAxvGoaEUaz+LY5UoDEv5OnCtaXT0zF/I
RU7Ah7lUG/nKFppASZK9b/Al20mK3b5V1Mw0v/Wk/Vwx1WsDFKsbRZHoefm8Stc=
=8zyQ
-----END PGP SIGNATURE-----