[Asterisk-code-review] app_waitforcond: New application (asterisk[16])

[N A]

Attention is currently required from: Kevin Harwell, Richard Mudgett.
N A has posted comments on this change. ( https://gerrit.asterisk.org/c/asterisk/+/15928 )

Change subject: app_waitforcond: New application
......................................................................


Patch Set 5:

(10 comments)

File apps/app_waitforcond.c:

https://gerrit.asterisk.org/c/asterisk/+/15928/comment/70192c13_1decb981 
PS1, Line 55: 			<parameter name="timeout" />
            : 			<parameter name="interval" />
> Add the unit type for these, and if seconds mention if fractions of a second are supported.
Done


https://gerrit.asterisk.org/c/asterisk/+/15928/comment/6db69965_a8d20f77 
PS1, Line 82: static int waitforcond_exec(struct ast_channel *chan, const char *data)
> You only want to return non-zero(-1) for applications if an error should be hanging up the call. […]
Done


https://gerrit.asterisk.org/c/asterisk/+/15928/comment/16137ddb_45a4c0fd 
PS1, Line 94: 	
> Remove whitespace
Done


https://gerrit.asterisk.org/c/asterisk/+/15928/comment/baf3c0d5_0429d34c 
PS1, Line 107: 	if (ast_strlen_zero(tmp) || tmp[0] == '\0') {
> The second check is already found in the ast_strlen_zero function so can be removed.
Done


https://gerrit.asterisk.org/c/asterisk/+/15928/comment/1cc48091_84df9662 
PS1, Line 113: 	if (!strchr(tmp, '[') && !strchr(tmp, ']')) {
> As you currently have it a string like "ab]cd[ef" would pass your check. […]
Done


https://gerrit.asterisk.org/c/asterisk/+/15928/comment/94944085_653641d6 
PS1, Line 203: 		cond = ast_strdupa(condition);
> Do not call strdupa within a loop as it could blow the stack. […]
Done


File apps/app_waitforcond.c:

https://gerrit.asterisk.org/c/asterisk/+/15928/comment/37373580_ca6eda01 
PS2, Line 103: 		ast_log(LOG_ERROR, "Channel already gone\n");
> This message is misleading.  The application is running on this channel. […]
Done


https://gerrit.asterisk.org/c/asterisk/+/15928/comment/32532119_a05240c9 
PS2, Line 104: 		return 0;
> Actually, this really should return -1. […]
Done


https://gerrit.asterisk.org/c/asterisk/+/15928/comment/19530c31_0700a6a4 
PS2, Line 199: 	
> whitespace
Done


File apps/app_waitforcond.c:

https://gerrit.asterisk.org/c/asterisk/+/15928/comment/16029adf_a080b3c7 
PS3, Line 55: 			<parameter name="timeout" />
            : 			<parameter name="interval" />
> Think this one got missed. […]
Done



-- 
To view, visit https://gerrit.asterisk.org/c/asterisk/+/15928
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: 16
Gerrit-Change-Id: I08adf2824b8bc63405778cf355963b5005612f41
Gerrit-Change-Number: 15928
Gerrit-PatchSet: 5
Gerrit-Owner: N A <mail at interlinked.x10host.com>
Gerrit-Reviewer: Friendly Automation
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>
Gerrit-CC: Joshua Colp <jcolp at sangoma.com>
Gerrit-Attention: Kevin Harwell <kharwell at digium.com>
Gerrit-Attention: Richard Mudgett <rmudgett at digium.com>
Gerrit-Comment-Date: Fri, 28 May 2021 17:30:19 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Kevin Harwell <kharwell at digium.com>
Comment-In-Reply-To: Richard Mudgett <rmudgett at digium.com>
Gerrit-MessageType: comment
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20210528/c05b8654/attachment.html>