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:09Date Closed:2006-09-09 16:51:45
Versions:Frequency of
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.


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.