[Asterisk-code-review] chan_dahdi: Add DAHDIBusyOut (asterisk[master])

[N A]

Attention is currently required from: Mark Murawski.

N A has posted comments on this change. ( https://gerrit.asterisk.org/c/asterisk/+/19264 )

Change subject: chan_dahdi: Add DAHDIBusyOut
......................................................................


Patch Set 6: Code-Review-1

(12 comments)

File channels/chan_dahdi.c:

https://gerrit.asterisk.org/c/asterisk/+/19264/comment/8dbac948_8306288c 
PS6, Line 132: #include "regex.h"
Is this supposed to be regex.h or <regex.h>?
If it's the latter, it should go above all header files except asterisk.h


https://gerrit.asterisk.org/c/asterisk/+/19264/comment/1c8dca70_66f506db 
PS6, Line 199: 			busy if BusyOut is enabled.</para>
The description needs to be expanded, namely clarification of what exactly is being answered. When it's enabled, the channel is answered? Or when somebody calls it, their call is answered? (That would set off some alarm bells in people's heads).


https://gerrit.asterisk.org/c/asterisk/+/19264/comment/99ac5a77_06bc4fe1 
PS6, Line 5934: 	  AST_APP_ARG(channel);
whitespacing is messed up here, and throughout this entire function


https://gerrit.asterisk.org/c/asterisk/+/19264/comment/db58dec6_0b5f08c4 
PS6, Line 8715:         if (p->busyout) {
whitespacing


https://gerrit.asterisk.org/c/asterisk/+/19264/comment/29c37964_5155a871 
PS6, Line 9820: 		ast_answer(chan);
If you're really going to answer, you should use ast_raw_answer instead.
Have you confirmed this does not work without calling ast_answer? And can you explain why we are answering, as a code comment?


https://gerrit.asterisk.org/c/asterisk/+/19264/comment/565d13b9_58184ea7 
PS6, Line 11855:                        }
whitespacing


https://gerrit.asterisk.org/c/asterisk/+/19264/comment/c8b7eabb_888b59f9 
PS6, Line 15858: 		return 1;
Again, I think returning 1 closes the manager session, this function should always return 0


https://gerrit.asterisk.org/c/asterisk/+/19264/comment/afb6d9e6_77e78f44 
PS6, Line 15880: 	       }
whitespacing in this function


https://gerrit.asterisk.org/c/asterisk/+/19264/comment/48dcaf1d_69956e96 
PS6, Line 16561: 
whitespacing


https://gerrit.asterisk.org/c/asterisk/+/19264/comment/06bff14b_45c06951 
PS6, Line 20335: 
whitespacing


https://gerrit.asterisk.org/c/asterisk/+/19264/comment/ca52a338_cd01e0c9 
PS6, Line 20362:                 usleep(1000);
I wonder if a thread to do this is the most efficient way.
Can you not hook into one of the existing DAHDI event loops?


https://gerrit.asterisk.org/c/asterisk/+/19264/comment/72283de9_89ae938f 
PS6, Line 20451: 		}
else should be on same line



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

Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Change-Id: Ib75c87c554bb18b8f65be73b4474474e7e1877b7
Gerrit-Change-Number: 19264
Gerrit-PatchSet: 6
Gerrit-Owner: Mark Murawski <markm at intellasoft.net>
Gerrit-Reviewer: Friendly Automation
Gerrit-Reviewer: N A <asterisk at phreaknet.org>
Gerrit-Attention: Mark Murawski <markm at intellasoft.net>
Gerrit-Comment-Date: Sat, 14 Jan 2023 14:07:13 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20230114/e788ea2b/attachment-0001.html>