[Asterisk-code-review] acl: implement a centralized ACL output mechanism for HAs and ACLs. (asterisk[master])

[Jaco Kroon]

Jaco Kroon has posted comments on this change. ( https://gerrit.asterisk.org/c/asterisk/+/14003 )

Change subject: acl: implement a centralized ACL output mechanism for HAs and ACLs.
......................................................................


Patch Set 3:

(7 comments)

> Patch Set 1: Code-Review-1

https://gerrit.asterisk.org/c/asterisk/+/14003/3/include/asterisk/acl.h 
File include/asterisk/acl.h:

https://gerrit.asterisk.org/c/asterisk/+/14003/3/include/asterisk/acl.h@433 
PS3, Line 433: #if defined(__cplusplus) || defined(c_plusplus)
             : }
             : #endif
> Shouldn't this be moved to after your new functions?
Done


https://gerrit.asterisk.org/c/asterisk/+/14003/3/include/asterisk/acl.h@451 
PS3, Line 451: void ast_ha_output(int fd, const struct ast_ha* ha, const char* prefix);
> There should be a space before the '*' for pointer types, not after.
Done


https://gerrit.asterisk.org/c/asterisk/+/14003/3/include/asterisk/acl.h@467 
PS3, Line 467: void ast_acl_output(int fd, struct ast_acl_list* acl, const char* prefix);
> Same as above.
Done


https://gerrit.asterisk.org/c/asterisk/+/14003/3/main/acl.c 
File main/acl.c:

https://gerrit.asterisk.org/c/asterisk/+/14003/3/main/acl.c@1087 
PS3, Line 1087: void ast_ha_output(int fd, const struct ast_ha* ha, const char* prefix)
> Space before '*' and not after.
Done


https://gerrit.asterisk.org/c/asterisk/+/14003/3/main/acl.c@1099 
PS3, Line 1099: void ast_acl_output(int fd, struct ast_acl_list* acl_list, const char* prefix)
> Same.
Done


https://gerrit.asterisk.org/c/asterisk/+/14003/3/main/manager.c 
File main/manager.c:

https://gerrit.asterisk.org/c/asterisk/+/14003/3/main/manager.c@2553 
PS3, Line 2553: 	if (user->acl && !ast_acl_list_is_empty(user->acl))
> ast_acl_list_is_empty is NULL safe, so the first check is not necessary.
Done


https://gerrit.asterisk.org/c/asterisk/+/14003/3/main/manager.c@2553 
PS3, Line 2553: if (user->acl && !ast_acl_list_is_empty(user->acl))
              : 		ast_acl_output(a->fd, user->acl, NULL);
> Should have braces
Done



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

Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Change-Id: I0ec9876a90dddd379c80ec078d48e3ee6991eb0f
Gerrit-Change-Number: 14003
Gerrit-PatchSet: 3
Gerrit-Owner: Jaco Kroon <jaco at uls.co.za>
Gerrit-Reviewer: Friendly Automation
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Joshua Colp <jcolp at sangoma.com>
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>
Gerrit-Reviewer: Sean Bright <sean.bright at gmail.com>
Gerrit-Comment-Date: Thu, 26 Mar 2020 18:36:39 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Sean Bright <sean.bright at gmail.com>
Gerrit-MessageType: comment
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20200326/8f27a7f7/attachment.html>