[Asterisk-code-review] Logging: Add debug logging categories (asterisk[master])

[George Joseph]

George Joseph has posted comments on this change. ( https://gerrit.asterisk.org/c/asterisk/+/14887 )

Change subject: Logging: Add debug logging categories
......................................................................


Patch Set 3: Code-Review-1

(3 comments)

https://gerrit.asterisk.org/c/asterisk/+/14887/3/include/asterisk/logger_category.h 
File include/asterisk/logger_category.h:

https://gerrit.asterisk.org/c/asterisk/+/14887/3/include/asterisk/logger_category.h@129 
PS3, Line 129: ast_debug_category_allow
ast_debug_is_category_allowed might be better.


https://gerrit.asterisk.org/c/asterisk/+/14887/3/include/asterisk/rtp_engine.h 
File include/asterisk/rtp_engine.h:

https://gerrit.asterisk.org/c/asterisk/+/14887/3/include/asterisk/rtp_engine.h@2838 
PS3, Line 2838: #define AST_DEBUG_CATEGORY_RTP (1 << 0) /* RTP debug logging category id */
              : #define AST_DEBUG_CATEGORY_RTP_PACKET (1 << 1) /* RTP packet debug logging category id */
              : #define AST_DEBUG_CATEGORY_RTCP (1 << 2) /* RTCP debug logging category id */
              : #define AST_DEBUG_CATEGORY_RTCP_PACKET (1 << 3) /* RTCP packet debug logging category id */
              : #define AST_DEBUG_CATEGORY_DTLS (1 << 4) /* DTLS debug logging category id */
              : #define AST_DEBUG_CATEGORY_DTLS_PACKET (1 << 5) /* DTLS packet debug logging category id */
              : #define AST_DEBUG_CATEGORY_ICE (1 << 8) /* ICE debug logging category id */
I think the problem now is that the next person who comes along and wants to add category debugging will need to search through all the source files to see what ids are in use/available.  Do we really need the capability to test multiple categories for a debug message?  If so, then I'd have the register function return an id to the module rather than the module providing one.  Alternatively, you could take a list of names to check.


https://gerrit.asterisk.org/c/asterisk/+/14887/3/include/asterisk/stun.h 
File include/asterisk/stun.h:

https://gerrit.asterisk.org/c/asterisk/+/14887/3/include/asterisk/stun.h@41 
PS3, Line 41: #define AST_DEBUG_CATEGORY_STUN (1 << 6) /* STUN debug logging category id */
            : #define AST_DEBUG_CATEGORY_STUN_PACKET (1 << 7) /* STUN packet debug logging category id */
            : 
This highlights my earlier comment about needing to know which ids are in use/available.



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

Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Change-Id: I6e6cb247bb1f01dbf34750b2cd98e5b5b41a1849
Gerrit-Change-Number: 14887
Gerrit-PatchSet: 3
Gerrit-Owner: Kevin Harwell <kharwell at digium.com>
Gerrit-Reviewer: Friendly Automation
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Comment-Date: Thu, 03 Sep 2020 12:31:54 +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/20200903/ba87670b/attachment.html>