[Nice] New API and refactor

Olivier Crête olivier.crete at collabora.co.uk
Thu May 8 17:10:34 PDT 2008


On Thu, 2008-05-08 at 09:38 +0300, Rémi Denis-Courmont wrote:
> Le Wednesday 07 May 2008 21:01:41 ext Olivier Crête, vous avez écrit :
>
> void stun_init_response (uint8_t *ans, size_t msize, const uint8_t *req)
> (...)
>   /* For RFC3489 compatibility, we cannot assume the cookie */
>   memcpy (ans + 4, req + 4, 4);
>   (void)stun_append_server (ans, msize);

Good example... look at the "stun_server_append" here... This is clearly
behavior specific.. and it should not be in the generic code...

Google Talk ignores stun packets that have unknown attributes, so
stun_fingerprint or stun-server can NOT be there.... I had to produce
theses horrors to make it possible to disable them:

http://git.collabora.co.uk/?p=user/tester/nice.git;a=commitdiff;h=1aeb122c0c558c5b2baa07cf92b77833ffe4b9fa
http://git.collabora.co.uk/?p=user/tester/nice.git;a=commitdiff;h=da1a1c216c4bb8e9c911fcc809afb0fddfb3c0e9

This should clearly be specific to a usage and not be in the core 
stuff...


> 
> > > >let me be clear, do we want to have the ICE agent use the STUN
> > > >library to build messages, then send them on the socket, and
> > > >when it receives a message, try to parse it with the STUN
> > > >library to see if it's a valid stun message, then handle it,
> > > >otherwise, give it to the application (first design)...
> > >
> > > [...]
> > >
> > If you using glib, any allocation that it does will use g_malloc(),
> > g_malloc() calls abort() if malloc fails.
> 
> Precisely, that's why the STUN code does not use glib.

This is about the agent (that does check the retval of g_slist_append()
everywhere)... 

Frankly, one of the main reasons we're going to rewrite it is that it is
almost impossible to follow what does what. Its filled with  magic
numbers without any comment to explain them with stuff like:
  h[0] = (c >> 1) | ((m >> 6) & 0x3e);
  h[1] = ((c << 4) & 0x10) | ((m << 1) & 0xe0) | (m & 0x0f);

I have no idea what that does or what its supposed to do.

And I won't mention that amount of seemingly random pointer arithmetic.

Unhappy Olivier
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: This is a digitally signed message part
Url : http://lists.freedesktop.org/archives/nice/attachments/20080508/b2edb71e/attachment.pgp 


More information about the Nice mailing list