On Tue, 04 Nov 2008 at 18:33:45 +0100, Senko Rasic wrote:
> Simon McVittie wrote:
> > g_slice_new0
>
> Changed in 79275e9c7b7da81465210292d3129e447626f260 .
> Since I'm creating the structs in media-stream and freeing them in
> jingle-transport-google and jingle-media-rtp, do you think I should
> have a helper function for creating them, and not having to rememer
> about g_slice in various places?
That might be useful, yes. I usually end up writing a foo_new() and a
foo_free() whenever a complicated struct foo exists.
> >> +#define SET_BAD_REQ(txt...) g_set_error (error, GABBLE_XMPP_ERROR,
> >> XMPP_ERROR_BAD_REQUEST, txt)
> >
> > I'd prefer this to take error as a first parameter so it isn't reliant
> > on naming.
>
> I'm using these as syntactic sugar because there are a lot of places
> where I have to return errors, so I thought the fewer characters the
> better (if i have to pass in XMPP_ERROR_{BAD_REQUEST,OUT_OF_ORDER,...}
> each time, there's not much typing saved :)
What I meant was (best viewed in monospace):
added this -----------\
|
v
#define SET_BAD_REQ(error, txt...) \
g_set_error (error, GABBLE_XMPP_ERROR, XMPP_ERROR_BAD_REQUEST, txt)
> >> + /* FIXME: report error to peer instead of assert */
> >> + g_assert (g_list_length (cs) == 1);
> >
> > Please don't assert on network traffic.
>
> It's not assertion on network traffic, it's a sanity check. Other parts
> of jingle code should ensure that session in google mode never != 1
> content (either by adding, removing, contents, or by putting
> multiple descriptions in session initiate action). If that happens,
> we're in serious problems anyways.
OK, if it's been checked already then an assert is fine.
> I've pushed these patches to my jingle repo. Content direction code,
> timeouts in content-add and <candidates> gtalk3 detection are still in
> the TODO list, I'll attack them now.
Great. Can we have a wiki page somewhere listing known problems with
this branch?
Simon
_______________________________________________
Telepathy mailing list
Telepathy@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/telepathy
