[Home]

Summary:ASTERISK-16817: ast_app_dtget inconsistency
Reporter:Dmitry Andrianov (dimas)Labels:
Date Opened:2010-10-16 09:13:39Date Closed:2012-01-20 18:23:01.000-0600
Priority:MinorRegression?No
Status:Closed/CompleteComponents:Core/General
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) app.c.patch-issue18146
Description:I noticed strange thing in the code decided to share. 1.6.2.13, main/app.c, ast_app_dtget function at the very beginning it tries to configure the timeout unless it is specified:

       if (!timeout && chan->pbx) {
               timeout = chan->pbx->dtimeoutms / 1000.0;
       } else if (!timeout) {
               timeout = 5;
       }

Both branches indicate  the final value for timeout is in seconds. However I believe ast_waitfordigit accepts timeout in milliseconds.
And as I see when ast_app_dtget is called by builtin_atxfer, the timeout passed is also in ms. So I guess the code above may be wrong.
Comments:By: Stefan Schmidt (schmidts) 2010-10-16 16:27:20

you are right, the timeout should not be in sec rather than ms but there is only a small chance this part of code will ever be used.
only 3 functions call ast_app_dtget, one of this (grab_transfer) use a fix param of 1000. The other both (builtin_blindxfer and atxfer) use transferdigittimeout as param which is set to 3000 per default or by user config.

atleast timeout is a int so doing a division by 1000.0 (which is a float value) could cause wrong results at all.

please try my attached patch.
By: Dmitry Andrianov (dimas) 2010-10-17 17:31:09

Just to be clear - the issue I reported does not cause anything in my setup. Because yes, it is not triggered. So I'm not reporting something which actually causes problem but rather just something which _looks_ wrong.

Your patch looks fine to me. But I see no point in testing because again - I do not have an issue on the first place.


By: Stefan Schmidt (schmidts) 2010-10-18 01:19:20

sorry about my bad english, what i mean was the timeout should be in ms not in sec ;)

ok i will set this to review.

thanks for notice this possible problem.