[Asterisk-code-review] func_groupcount.c: Adding Group Variables + additional Group functions (asterisk[18])

[N A]

Attention is currently required from: Mark Murawski.

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

Change subject: func_groupcount.c: Adding Group Variables + additional Group functions
......................................................................


Patch Set 22:

(3 comments)

File main/app.c:

https://gerrit.asterisk.org/c/asterisk/+/17655/comment/6e6551a2_f8bab07c 
PS21, Line 2444: 	AST_RWLIST_TRAVERSE_SAFE_BEGIN(&groups_meta, gmi, group_meta_list) {
> Same this as below... being explicit about what we're looking for. […]
It doesn't, if you want to leave it that's fine, I just thought that was preferred from a project perspective.


https://gerrit.asterisk.org/c/asterisk/+/17655/comment/938f2f98_48965039 
PS21, Line 2479: 	if (newvariable == NULL) {
> > Could just do !newvariable […]
Same thing here, ISTR that in the coding guidelines, but not a big deal either way


https://gerrit.asterisk.org/c/asterisk/+/17655/comment/37ca9399_d9890dc3 
PS21, Line 2854: int ast_app_group_meta_wrlock(void)
> > I must be missing something, but why are these functions here? I see them in the header file, and  […]
But there are currently no users of these APIs, anywhere?
It just seems unnecessary to expose these outside of the file, for no particular good reason. If misused, it could easily lead to locking issues since you're letting people selective unlock and lock different things in potentially invalid ways.

If they're not being used by anything and there's not a reason for them to exist, I think they should be removed.



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

Gerrit-Project: asterisk
Gerrit-Branch: 18
Gerrit-Change-Id: I23e48d1cdfc8adaffdfec2e936e56143603914f2
Gerrit-Change-Number: 17655
Gerrit-PatchSet: 22
Gerrit-Owner: Mark Murawski <markm at intellasoft.net>
Gerrit-Reviewer: Friendly Automation
Gerrit-Reviewer: N A <asterisk at phreaknet.org>
Gerrit-Reviewer: Sean Bright <sean at seanbright.com>
Gerrit-CC: George Joseph <gjoseph at digium.com>
Gerrit-Attention: Mark Murawski <markm at intellasoft.net>
Gerrit-Comment-Date: Sat, 14 Jan 2023 21:08:06 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: N A <asterisk at phreaknet.org>
Comment-In-Reply-To: Mark Murawski <markm at intellasoft.net>
Gerrit-MessageType: comment
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20230114/91f65840/attachment-0001.html>