[Home]

Summary:ASTERISK-02810: [patch] Line 227 & 228 in res_musiconhold.c wtf are we closing really?
Reporter:Brian West (bkw918)Labels:
Date Opened:2004-11-14 01:17:16.000-0600Date Closed:2008-01-15 15:14:18.000-0600
Priority:MajorRegression?No
Status:Closed/CompleteComponents:Resources/res_musiconhold
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) res_musiconhold.diff.txt
Description:I can't see where in res_musiconhold where we need to call close that many times.  Valgrind really really really complains about it... doesn't look right to me but whats the right fix?
Comments:By: Mark Spencer (markster) 2004-11-14 19:43:16.000-0600

We fork.  If we don't close the file descriptors then if Asterisk terminates, the mpg123 process will keep it's FD's including sockets etc open for as long as it's running which is a Very Bad Thing(tm).  I do not know of a system call that will tell us the highest open file descriptor so we have to close a bunch.
By: Brian West (bkw918) 2004-11-14 20:56:45.000-0600

After talking with fOSSil about this he recommended this... :P
By: Paul Cadach (pcadach) 2004-11-17 11:45:13.000-0600

Just one additional syscall to make valgrind happy... Is it really required?
By: Clod Patry (junky) 2004-11-17 12:21:53.000-0600

in almost all * code, all condition are made is this way:
if ( value != digit) {
so if i can suggest, it would be cool to be consistent, no?

so
if (-1 != fcntl(x, F_GETFL)) {

could be replaced by:
if (fcntl(x, F_GETFL)!= -1) {
By: Brian West (bkw918) 2004-11-17 12:55:35.000-0600

this is just done at start up.

bkw
By: Mark Spencer (markster) 2004-11-17 13:15:48.000-0600

Fixed in CVS
By: Russell Bryant (russell) 2004-11-17 23:20:07.000-0600

fixed in 1.0 - will be in 1.0.3
By: Digium Subversion (svnbot) 2008-01-15 15:14:10.000-0600

Repository: asterisk
Revision: 4279

U   trunk/res/res_musiconhold.c

------------------------------------------------------------------------
r4279 | markster | 2008-01-15 15:14:09 -0600 (Tue, 15 Jan 2008) | 2 lines

Check that FD's are open before closing (bug ASTERISK-2810)

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=4279
By: Digium Subversion (svnbot) 2008-01-15 15:14:18.000-0600

Repository: asterisk
Revision: 4289

U   branches/v1-0/res/res_musiconhold.c

------------------------------------------------------------------------
r4289 | russell | 2008-01-15 15:14:17 -0600 (Tue, 15 Jan 2008) | 2 lines

check to see if FD's are open before closing (bug ASTERISK-2810)

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=4289