[Home]

Summary:ASTERISK-04907: [patch] Remove extra ast_mutex_unlock() in chan_vpb.c
Reporter:knielsen (knielsen)Labels:
Date Opened:2005-08-31 03:52:15Date Closed:2005-08-31 21:21:47
Priority:MinorRegression?No
Status:Closed/CompleteComponents:Core/General
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) chan_vpb_patch_1.txt
Description:I stumbled upon this small typo in channels/chan_vpb.c.

The vpb_hangup() function has an error case that unlocks a mutex that is not locked. This clearly happened when the locking of the mutex was commented out in revision 1.34.

Attached one-line patch fixes this. Not sure if this can occur in practice (don't use chan_vpb), but thought I would report it now that I saw it.


****** ADDITIONAL INFORMATION ******

Discalimer is on file
Comments:By: Kevin P. Fleming (kpfleming) 2005-08-31 12:17:59

There is at least one more unlock call in this function that is not addressed by your patch, but unless you can prove both of them are not needed I'm hesitant to remove them. It would be better to notify the maintainer at Voicetronix so he can review the situation.
By: knielsen (knielsen) 2005-08-31 14:36:53

Not sure what you mean; the code reads essentially

static int vpb_hangup(struct ast_channel *ast)
{
/*
ast_mutex_lock(&p->lock);
*/
if (!ast->tech || !ast->tech_pvt) {
res = ast_mutex_unlock(&p->lock);
return 0;
}

It seems pretty obvious to me that when the ast_mutex_lock() call was commented out in 1.34 it was forgotten to comment out the ast_mutex_unlock() call. The other lock/unlock calls in that function match up nicely.

Anyway, feel free to do as you please. I don't use chan_vpb, don't even know what it is :-) I just happened to spot this while reviewing hangup code for bug 5051, and didn't want to just ignore it.
By: Kevin P. Fleming (kpfleming) 2005-08-31 17:24:16

Assigned to the chan_vpb maintainer for review.
By: VoiceTronix (benkramer) 2005-08-31 18:24:22

They probably can be removed. But as it is at the end of a call, it isnt going to hurt to unlock a something that may not be locked. I cant test it at the moment, as I am currently re-writing the channel driver, basically from scratch.