[Home]

Summary:ASTERISK-08278: [patch] voicemail with volgain leaves behind temp file
Reporter:Richard Miller (ulogic)Labels:
Date Opened:2006-12-04 21:29:36.000-0600Date Closed:2007-06-21 14:56:48
Priority:MinorRegression?No
Status:Closed/CompleteComponents:Applications/app_voicemail
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) app_voicemail.patch
( 1) app_voicemail-1.2.13.patch
Description:This is an update to issue 0006237 which is closed.
This is in the sendmail() function in app_voicemail.c
A temporary file is created with mkstemp, but when executing sox, the base file name has an extension appended for the output file name, and that file is not deleted.



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

This is how I dealt with it.  (I applied the volgain differences in the beta version to my production system running version 1.2.13, but these changes apply to the 1.4 beta3 version as well).  Sorry I did not provide a diff file (I'm a relative newbie in Linux), but these are the changes in the sendmail() function in the area around lines 1974-2010 in the 1.4 beta3 version from 2006-09-27.

old
char tmpdir[256], newtmp[256];
int tmpfd;
new
char tmpdir[256], newtmp[256], *newext;
int tmpfd = -1;

old
create_dirpath(tmpdir, sizeof(tmpdir), vmu->context, vmu->mailbox, "tmp");
snprintf(newtmp, sizeof(newtmp), "%s/XXXXXX", tmpdir);
tmpfd = mkstemp(newtmp);
ast_log(LOG_DEBUG, "newtmp: %s\n", newtmp);
if (vmu->volgain < -.001 || vmu->volgain > .001) {
new
if (vmu->volgain < -.001 || vmu->volgain > .001) {
create_dirpath(tmpdir, sizeof(tmpdir), vmu->context, vmu->mailbox, "tmp");
snprintf(newtmp, sizeof(newtmp), "%s/XXXXXX", tmpdir);
tmpfd = mkstemp(newtmp);
ast_log(LOG_DEBUG, "newtmp: %s\n", newtmp);

old
if (tmpfd > -1)
close(tmpfd);
unlink(newtmp);
new
/* Remove temporary files, both with and without the extension */
if (tmpfd > -1) {
newext=memchr(newtmp, 0, sizeof newtmp);
sprintf(newext, ".%s", format);
unlink(newtmp);
*newext = 0;
close(tmpfd);
unlink(newtmp);
}
Comments:By: Richard Miller (ulogic) 2006-12-04 22:00:47.000-0600

After looking back over the code, the last change was doing some redundant work.

Remove declaration of *newext;

Change cleanup code to:
  /* Remove temporary files, both with and without the extension */
   if (tmpfd > -1) {
       unlink(fname);
       close(tmpfd);
       unlink(newtmp);
   }

By: jmls (jmls) 2006-12-05 04:32:14.000-0600

ulogic, would you please send the patch as an attachment, against the latest svn trunk of 1.4. Do you have a disclaimer on file ? If not, we need one sending or faxing to digium before anyone can look at this patch. thanks.

By: Serge Vecher (serge-v) 2006-12-05 08:34:03.000-0600

See bottom of http://bugs.digium.com/main_page.php) for disclaimer information.
See http://www.asterisk.org/developers/Patch_Howto for the patch how-to.

By: Richard Miller (ulogic) 2006-12-05 08:45:24.000-0600

I have just sent in the disclaimer.  Please bear with me as I have never submitted a patch before.  I am reading the procedures for submitting a patch file and will have it later today.

By: Richard Miller (ulogic) 2006-12-05 09:45:33.000-0600

I have just uploaded the patch for the 1.4 beta3 version correcting the issue.  I have also uploaded a patch for the 1.2.13 version I am using on my production system which incorporates the volgain changes.

By: Jason Parker (jparker) 2006-12-05 10:25:55.000-0600

Without looking at the code, I believe this is partially related to 8016, so I'm going to mark it as such.  Chances are, the same fix will affect both bugs (possibly negatively...so please be mindful of 8016 when writing a patch for this).

By: Leif Madsen (lmadsen) 2007-04-03 12:06:30

/Housekeeping

What do we need to do to move this bug forward? Apparently it might break the patch for 8016, but 8016 is working fine for several people.

Do we commit 8016 first since its a trivial change and then try to fix this one after the fact?

Leif.

By: Mark Michelson (mmichelson) 2007-06-21 14:56:46

I tested this patch and have committed the changes. This is fixed in 1.4 in SVN rev 70808 and in trunk in rev 70809.

I have not made changes to 1.2 since that would constitute adding a new feature to a stable branch, which is not allowed.