Summary: | ASTERISK-05901: useless code ? | ||
Reporter: | Luigi Rizzo (rizzo) | Labels: | |
Date Opened: | 2005-12-24 18:54:04.000-0600 | Date Closed: | 2008-01-15 16:47:54.000-0600 |
Priority: | Trivial | Regression? | No |
Status: | Closed/Complete | Components: | Channels/chan_sip/General |
Versions: | Frequency of Occurrence | ||
Related Issues: | |||
Environment: | Attachments: | ||
Description: | chan_sip.c:: do_monitor() has, near the middle, the following code which is highly dubious, because it is executed without holding any lock nor it does anything in the body. Unless someone can explain what it is doing, i think it should go. /* Don't let anybody kill us right away. Nobody should lock the interface list and wait for the monitor list, but the other way around is okay. */ ast_mutex_lock(&monlock); /* Lock the network interface */ ast_mutex_lock(&netlock); /* Okay, now that we know what to do, release the network lock */ ast_mutex_unlock(&netlock); /* And from now on, we're okay to be killed, so release the monitor lock as well */ ast_mutex_unlock(&monlock); | ||
Comments: | By: Olle Johansson (oej) 2006-01-03 04:44:41.000-0600 I can't explain it so I duck by assigning this to Kevin ;-) By: Matthew Fredrickson (mattf) 2006-01-31 17:18:23.000-0600 Poke Kevin, any thoughts on this? By: Kevin P. Fleming (kpfleming) 2006-02-14 18:56:18.000-0600 Agreed, my analysis of the usage of these locks does not show that this is accomplishing anything... so it's gone from trunk. I did not remove it from branch-1.2 though, since there is the remote possibility it is providing some useful purpose and the kind of problems it could be hiding are too serious to inflict on a release branch :-) By: Digium Subversion (svnbot) 2008-01-15 16:47:54.000-0600 Repository: asterisk Revision: 10142 U trunk/channels/chan_sip.c ------------------------------------------------------------------------ r10142 | kpfleming | 2008-01-15 16:47:54 -0600 (Tue, 15 Jan 2008) | 2 lines remove code that does not appear to do anything useful (issue ASTERISK-5901) ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=10142 |