[Asterisk-code-review] stasis.c: Added topic_all container (...asterisk[16])

[Joshua Colp]

Joshua Colp has posted comments on this change. ( https://gerrit.asterisk.org/c/asterisk/+/11013 )

Change subject: stasis.c: Added topic_all container
......................................................................


Patch Set 7: Code-Review-1

(9 comments)

https://gerrit.asterisk.org/#/c/11013/7/include/asterisk/stasis.h 
File include/asterisk/stasis.h:

https://gerrit.asterisk.org/#/c/11013/7/include/asterisk/stasis.h@524 
PS7, Line 524:  * \param uniqueid Uniqueid of the new topic.
This isn't passed in.


https://gerrit.asterisk.org/#/c/11013/7/include/asterisk/stasis.h@526 
PS7, Line 526:  * \param detail Detail of the new topic.
What does detail mean exactly? What should be placed in here?


https://gerrit.asterisk.org/#/c/11013/7/include/asterisk/stasis.h@546 
PS7, Line 546: struct stasis_topic *stasis_topic_get(const char *uniqueid);
This passes in name now, so it should be named appropriately.


https://gerrit.asterisk.org/#/c/11013/7/main/stasis.c 
File main/stasis.c:

https://gerrit.asterisk.org/#/c/11013/7/main/stasis.c@435 
PS7, Line 435: 	ast_log(LOG_DEBUG, "Destroying topic. name: %s, detail: %s\n",
This should use ast_debug with a sufficiently high debug level


https://gerrit.asterisk.org/#/c/11013/7/main/stasis.c@491 
PS7, Line 491: 	if (!topic || !name || !detail) {
What if name is empty?


https://gerrit.asterisk.org/#/c/11013/7/main/stasis.c@557 
PS7, Line 557: 	if (!name || !detail) {
What if name is empty?


https://gerrit.asterisk.org/#/c/11013/7/main/stasis.c@560 
PS7, Line 560: 	ast_log(LOG_DEBUG, "Creating topic. name: %s, detail: %s\n", name, detail);
This should use ast_debug with a sufficiently high debug level


https://gerrit.asterisk.org/#/c/11013/7/main/stasis.c@564 
PS7, Line 564: 		ast_log(LOG_DEBUG, "Topic is already exist. name: %s, detail: %s\n",
This should use ast_debug with a sufficiently high debug level


https://gerrit.asterisk.org/#/c/11013/7/main/stasis.c@1184 
PS7, Line 1184: tmp_subscription = AST_VECTOR_GET_CMP(&topic->subscribers, sub, TOPIC_SUBSCRIPTION_CMP);
              : 	if (!tmp_subscription) {
              : 		AST_VECTOR_APPEND(&topic->subscribers, sub);
              : 	}
This seems like a separate bug fix, and did you encounter a case where this happened?



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

Gerrit-Project: asterisk
Gerrit-Branch: 16
Gerrit-Change-Id: Ie86d125d2966f93de74ee00f47ae6fbc8c081c5f
Gerrit-Change-Number: 11013
Gerrit-PatchSet: 7
Gerrit-Owner: sungtae kim <pchero21 at gmail.com>
Gerrit-Reviewer: Corey Farrell <git at cfware.com>
Gerrit-Reviewer: Friendly Automation
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: sungtae kim <pchero21 at gmail.com>
Gerrit-Comment-Date: Thu, 21 Mar 2019 13:28:28 +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/20190321/fc818c38/attachment.html>