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

[kobaz]

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

(Updated 2010-11-21 11:32:58.937970)


Review request for Asterisk Developers.


Changes
-------

Fixed cleanup condition.  There are two cases in which we won't have a user_no when jumping into outrun.  The user ref is always cleaned up since that's allocated first.  The container and related items are only cleaned up if the container has been set up.

Now, the main differences are: the user ref is always cleaned up, the misuse of the user pointer is fixed, and cleanups will run in the event of the noted ao2 alloc failures.


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 (updated)
-----

  trunk/apps/app_meetme.c 295784 

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/68fc1d0d/attachment.htm