Summary: | ASTERISK-07697: [patch] fix infinite loop in app_record when using '%d' option | ||
Reporter: | cmaj (cmaj) | Labels: | |
Date Opened: | 2006-09-08 08:58:09 | Date Closed: | 2006-09-09 16:51:45 |
Priority: | Blocker | Regression? | No |
Status: | Closed/Complete | Components: | Applications/app_record |
Versions: | Frequency of Occurrence | ||
Related Issues: | |||
Environment: | Attachments: | ( 0) app_record_infinite_loop.diff.txt | |
Description: | The ast_fileexists check was incorrectly used as the condition of a while loop, so that the initial recording of a file with the '%d' option always failed and thus remained in the loop. This resulted in a blocked channel every time. ****** ADDITIONAL INFORMATION ****** I see that ast_fileexists checks are performed throughout the code in a non-standard way, ie. == -1, > 0, != -1, etc. I saw just about every permutation imaginable. This probably produced a little confusion that led to the introduction of the bug, in addition to the '%d' option being a non-standard application option. | ||
Comments: | By: Tilghman Lesher (tilghman) 2006-09-08 13:10:30 Did this flag never work? Or did the recent change cause this issue? By: cmaj (cmaj) 2006-09-08 14:25:38 No, it used to work, at least with 1.2 stable, as far as I remember. I just used it in trunk for the first time yesterday and found it broken. So I don't know when it happened. Can you reproduce the bug in trunk ? By: Tilghman Lesher (tilghman) 2006-09-08 15:31:42 I made a change yesterday, to deal with a format string vulnerability. By: cmaj (cmaj) 2006-09-08 16:25:55 I agree that patch for 7811 was a major improvement, now that I look at it, so thanks for that. Problem is that bug was covering up this bug. I keep tracing back ast_fileexists, and it appears to only return -1 as the default condition when there's something like a file system error. It's not documented, but then it appears to return 0 if the file does not exist and 1 if it does exist. Maybe it should just test for existence, then ? Like: while ( ast_fileexists(tmp, ext, chan->language) ) By: Tilghman Lesher (tilghman) 2006-09-09 16:51:45 Fixed in 42621. Thanks for the detailed explanation. |