[Asterisk-code-review] pbx dundi: Added IPv6 support for dundi (asterisk[13])

Matthew Fredrickson asteriskteam at digium.com
Mon Aug 28 09:11:42 CDT 2017


Matthew Fredrickson has posted comments on this change. ( https://gerrit.asterisk.org/6173 )

Change subject: pbx_dundi: Added IPv6 support for dundi
......................................................................


Patch Set 1: Code-Review-1

(2 comments)

https://gerrit.asterisk.org/#/c/6173/1/pbx/pbx_dundi.c
File pbx/pbx_dundi.c:

https://gerrit.asterisk.org/#/c/6173/1/pbx/pbx_dundi.c@253
PS1, Line 253:     union ip_addr  addr;                            /*!< Other end of transaction */
Nitpick, but keeping indentation styles with the existing file is usually best practice when making a contribution to another project.  Asterisk generally indents using tabs, not spaces.


https://gerrit.asterisk.org/#/c/6173/1/pbx/pbx_dundi.c@439
PS1, Line 439:         if ( netsock_ai_family == AF_INET6)
Another nitpick, following existing coding styles is usually best practice when contributing.  So typically, opening brackets for conditionals should be on the same lines as the conditional.  Also, preceding whitespace between the '(' character and the beginning of the conditional element "if ( netsock_ai_family" is not convention in Asterisk's code - instead should be "if (netsock_ai_family".

For example, it should probably look like this:
if (netsock_ai_family == AF_INET6) {
   ....
} else {
   ....
}

You actually might want to look at the coding guidelines wiki page at:
https://wiki.asterisk.org/wiki/display/AST/Coding+Guidelines



-- 
To view, visit https://gerrit.asterisk.org/6173
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifbe298afc6416ba400db7be404a25994ad23968a
Gerrit-Change-Number: 6173
Gerrit-PatchSet: 1
Gerrit-Owner: Adam Secombe <adam.j.secombe at boeing.com>
Gerrit-Reviewer: Jenkins2
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Matthew Fredrickson <creslin at digium.com>
Gerrit-Comment-Date: Mon, 28 Aug 2017 14:11:42 +0000
Gerrit-HasComments: Yes
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20170828/c2762d02/attachment.html>


More information about the asterisk-code-review mailing list