[Asterisk-code-review] core/pbx: dialplan show - display filename/line# (asterisk[master])

[Corey Farrell]

Corey Farrell has posted comments on this change. ( https://gerrit.asterisk.org/4633 )

Change subject: core/pbx: dialplan show - display filename/line#
......................................................................


Patch Set 1: Code-Review-1

(4 comments)

https://gerrit.asterisk.org/#/c/4633/1/include/asterisk/pbx.h
File include/asterisk/pbx.h:

Line 529: int ast_add_extension2_with_lines(struct ast_context *con, int replace, const char *extension,
Do you plan to backport this to 13/14?  If not I think it would be better to just add parameters to ast_add_extension2 / ast_add_extension2_nolock.  We already have multiple variations of this function and 'grep -e ast_add_extension' only matches 24 lines outside of pbx.c / pbx.h.


Line 1302: /*! 
Red


Line 1314:  * \retval 0 or positive integer indicating what line in the config file was responsible for
Do we want '0' to be a valid line number or could this represent lack of line number?  If someone were to check the result of this function it would be easier to just check if it's true/false.  Given my comment about adding registrar_file/registrar_line to existing ast_add_extension2 it would be more natural for the caller to specify "NULL, 0" when not providing the config details.


https://gerrit.asterisk.org/#/c/4633/1/main/pbx.c
File main/pbx.c:

PS1, Line 655: 	const char *registrar, int lock_context,
             : 	const char *registrar_file, int registrar_line);
Nit: could we keep 'lock_context' last in the parameter list?  This will make it easier to read compared to ast_add_extension2 / ast_add_extension2_nolock.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ieccc6abccdff34ed5c7da3511fd24972b8f2dd30
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: Jonathan R. Rose <jonathan.rose at motorolasolutions.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: Corey Farrell <git at cfware.com>
Gerrit-HasComments: Yes