[Nice] Patch for TURN CreatePermission

[Madaro Livio]

Thanks for reviewing and fixing the patch. I had a look at the final result and I think it's OK.

Regards,
Livio
 

-----Original Message-----
From: nice-bounces+livio.madaro=telecomitalia.it at lists.freedesktop.org [mailto:nice-bounces+livio.madaro=telecomitalia.it at lists.freedesktop.org] On Behalf Of Youness Alaoui
Sent: martedì 20 novembre 2012 0.22
To: nice at lists.freedesktop.org
Subject: Re: [Nice] Patch for TURN CreatePermission

Thanks,

I had a couple of issues with the patch, but I fixed them and committed, have a
look at the final result :
http://cgit.collabora.com/git/libnice.git/commit/?id=d977c25410d89ab9028228c247b7751a965a0fb4

Changes :
- Removed trailing whitespaces and other small coding style inconsistencies
- Used the GList iterator in a deletion-safe way without using get_nth
- renamed variable to be less than 100 characters long :p


On 11/09/2012 08:37 AM, Madaro Livio wrote:
> OK. I think you are right. I remove current_create_permission_msg and I fix the memory leak issue.
> 
> I send the new patch for review.
> 
> Livio
> 
> 
> -----Original Message-----
> From: nice-bounces+livio.madaro=telecomitalia.it at lists.freedesktop.org [mailto:nice-bounces+livio.madaro=telecomitalia.it at lists.freedesktop.org] On Behalf Of Youness Alaoui
> Sent: giovedì 8 novembre 2012 19.46
> To: nice at lists.freedesktop.org
> Subject: Re: [Nice] Patch for TURN CreatePermission
> 
> Hi Madaro,
> 
> Sorry for not replying earlier, I was travelling.
> Thanks for the patch. That's a good point you got there, however, the patch kept
> the priv->current_create_permission_msg, but I think it should be removed. There
> is no reason to keep the message twice (in the list and in the current_*) and it
> gets confusing, you do a delete_link and g_free the
> current_create_permission_msg in the code, but in the socket_close, you removed
> the g_free.. also, you call g_list_free but you should call g_list_free_full in
> order to also free the messages contained within the list, otherwise you get a
> memleak.
> I don't have time right now to fix all of that myself, could you redo the patch
> and send me a new one for review?
> 
> Thank you,
> Youness.
> 
> On 11/02/2012 08:59 AM, Madaro Livio wrote:
>> Hi, I found a problem using TURN.
>>
>> Libnice sends a TURN message CreatePermission Request for each remote candidate
>> added by nice_agent_set_remote_candidates. The TURN server replies with a
>> CreatePermission Response and libnice matches the Request and the Response using
>> the message id. The problem is that libnice uses a single variable to store the
>> sent messages (current_create_permission_msg).If a new CreatePermission Request
>> is sent before receiving the Response of the previous one, the variable
>> current_create_permission_msgis overwritten and the Response of the first
>> message will never match the Request.
>>
>>
>>
>> I send a patch for review. It adds a GList variable
>> (pending_create_permission_messages) to store all the CreatePermission Request
>> waiting for a Response.
>>
>>
>>
>> Regards,
>>
>> Livio
>>
>>
>>
>> Questo messaggio e i suoi allegati sono indirizzati esclusivamente alle persone
>> indicate. La diffusione, copia o qualsiasi altra azione derivante dalla
>> conoscenza di queste informazioni sono rigorosamente vietate. Qualora abbiate
>> ricevuto questo documento per errore siete cortesemente pregati di darne
>> immediata comunicazione al mittente e di provvedere alla sua distruzione, Grazie.
>>
>> /This e-mail and any attachments// is //confidential and may contain privileged
>> information intended for the addressee(s) only. Dissemination, copying, printing
>> or use by anybody else is unauthorised. If you are not the intended recipient,
>> please delete this message and any attachments and advise the sender by return
>> e-mail, Thanks./
>>
>> *rispetta l'ambienteRispetta l'ambiente. Non stampare questa mail se non è
>> necessario.*
>>
>>
>>
>> _______________________________________________
>> nice mailing list
>> nice at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/nice
>>
> 
> 
> 
> Questo messaggio e i suoi allegati sono indirizzati esclusivamente alle persone indicate. La diffusione, copia o qualsiasi altra azione derivante dalla conoscenza di queste informazioni sono rigorosamente vietate. Qualora abbiate ricevuto questo documento per errore siete cortesemente pregati di darne immediata comunicazione al mittente e di provvedere alla sua distruzione, Grazie.
> 
> This e-mail and any attachments is confidential and may contain privileged information intended for the addressee(s) only. Dissemination, copying, printing or use by anybody else is unauthorised. If you are not the intended recipient, please delete this message and any attachments and advise the sender by return e-mail, Thanks.
> 
> 
> 
> _______________________________________________
> nice mailing list
> nice at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/nice
>