[Bug 28367] Add StreamTube interface support

[bugzilla-daemon at freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=28367

--- Comment #8 from Dario Freddi <drf54321 at gmail.com> 2010-06-11 03:11:50 PDT ---
(In reply to comment #7)
> It's starting to be in a really good shape... But I'm still going to bother you
> with some small finishing touches :P

Hehe, np, I'm as picky as hell, so I expect others to be as picky as me :P

> 
> One thing I previously didn't notice is you should have pretty include headers
> (TelepathyQt4/ClassName) for all of your public classes. Currently there seems
> to be one for just the "main class" of a header (eg.
> IncomingStreamTubeChannel). You should also have pretty include headers for the
> "auxiliary" classes like PendingStreamTubeConnection, obviously just including
> the same header as the main include, in this case
> incoming-stream-tube-channel.h.

Correct. Added in 8cbaff8dbcd862056b75a086405cd4a5b645336d.

> 
> IncomingStreamTubeChannel::acceptTubeAsTcpSocket(): You allow the case of
> allowedAddress != Any but allowedPort == 0 - which doesn't make sense, and
> MIGHT fail on the CM, but not necessarily, and would then only fail obscurely
> when trying to actually connect to the CM socket. So either error out if an
> address is provided but not a port, or better yet don't have default parameters
> but a separate overload taking neither and specifying Any, 0 - leading to you
> having to specify exactly neither or both.

This is correct. After thinking about it a small while, I came to the
conclusion that having a separate overload would make more sense. However, to
save code and make things more consistent, I also fixed the behavior of the
other overload (check if port is valid). This way, the overload with no
parameters just calls acceptTubeAsTcpSocket(QHostAddress::Any, 0). I also
specified in the docs all the possible behaviors and how the two overloads
interact. This is in 89e1787569842039d8d8e899b1fa413d8590cd71.

> 
> With these, I'm good for a merge (phew).

\o/!!

> Andre will likely want to review this
> stuff as well though - so stay tuned for his comments.

Yup, level 2 incoming :)

> In the meantime, I'll
> move on to looking at your dbus stuff. Is there a chance the improvements >we've
> done here would be applicable there too btw? You know what to do if so ;)

Possibly. Check your mail in the next hours to be sure ;)

-- 
Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.
You are the assignee for the bug.