[asterisk-dev] [Code Review] app_meetme.c conf_run() leaks refs

kobaz reviewboard at asterisk.org
Sat Nov 20 22:19:51 CST 2010


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/1025/
-----------------------------------------------------------

Review request for Asterisk Developers.


Summary
-------

There's two conditions (original lines 2626, 3675) where in the unlikly event of an ao2 alloc failing, we bail from conf_run and never do any cleanup.  This leaks a ref to an ast_conf_user.

Also there's a problem with a misuse of the user* pointer while in the *8 submenu (original line 3209), we lose the original *user pointer which will prevent proper cleanup on leave.

And lastly, this part at the bottom looks like it will never run.  It looks like if we get to this point, user->user_no will always have been properly initialized... and why would we want to decrement the ref count only if user_no is invalid?  We should always clean up at the end of this function.

3699		if (!user->user_no) {
3700			ao2_ref(user, -1);

So far, I think that everything in the else block of the above if could be always done on cleanup.


Diffs
-----

  trunk/apps/app_meetme.c 295123 

Diff: https://reviewboard.asterisk.org/r/1025/diff


Testing
-------

-- join
0xba69cb8 =1   app_meetme.c:1204:build_conf ()
0xb970d38 =1   app_meetme.c:2247:conf_run ()
0xb970d38 +1   app_meetme.c:2427:conf_run () [@1]
-- leave
0xb970d38 -1   app_meetme.c:3753:conf_run () [@2]
0xb970d38 -1   app_meetme.c:3756:conf_run () [@1]
0xba69cb8 -1   app_meetme.c:1849:conf_free () [@1]
0xba69cb8 **call destructor** app_meetme.c:1849:conf_free ()


Thanks,

kobaz

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.digium.com/pipermail/asterisk-dev/attachments/20101121/0457c494/attachment.htm 


More information about the asterisk-dev mailing list