[Asterisk-code-review] logger: Add custom logging capabilities (asterisk[master])

[Joshua Colp]

Attention is currently required from: N A.
Joshua Colp has posted comments on this change. ( https://gerrit.asterisk.org/c/asterisk/+/16222 )

Change subject: logger: Add custom logging capabilities
......................................................................


Patch Set 5: Code-Review-1

(7 comments)

File configs/samples/logger.conf.sample:

https://gerrit.asterisk.org/c/asterisk/+/16222/comment/e693c24d_f5b59c08 
PS5, Line 89: ; be sent to logging channels. The maximum number of dynamic
Dynamic levels is an internal concept that a user may not know. It's best to use the same language here, for example:

The maximum number of custom logging levels is 16, but not all of these may be available if modules in Asterisk define their own.


File doc/CHANGES-staging/logger.txt:

https://gerrit.asterisk.org/c/asterisk/+/16222/comment/d891d330_c73b234e 
PS5, Line 3: Adds the ability for custom dynamic logging
Stick to custom logging levels.


File include/asterisk/logger.h:

https://gerrit.asterisk.org/c/asterisk/+/16222/comment/fbb83fe2_1ced940c 
PS5, Line 336:  * \retval non-zero negative to be used with ast_log_dynamic_level for sending messages to this level
I don't understand this, non-zero negative? What happens with zero? -1 is a non-zero negative as well


https://gerrit.asterisk.org/c/asterisk/+/16222/comment/d96a6663_7ec9f704 
PS5, Line 338: int ast_dynamic_logger_level(const char *name);
This should be ast_logger_dynamic_level to match the API naming scheme already present.


File main/logger.c:

https://gerrit.asterisk.org/c/asterisk/+/16222/comment/4a066d44_803b71be 
PS5, Line 819: 			char *customlog = ast_strdup(logfile);
Why does this need to be duplicated? logger_register_level internally duplicates and stores it. Right now this will cause a leak of customlog.


https://gerrit.asterisk.org/c/asterisk/+/16222/comment/36855785_a5be9db9 
PS5, Line 2505: 	unsigned int available = 0;
You've defined this as an unsigned int, but logger_register_level returns an int.


https://gerrit.asterisk.org/c/asterisk/+/16222/comment/b0e32768_b9d41f94 
PS5, Line 2519: 	AST_RWLIST_WRLOCK(&logchannels);
This can be a RDLOCK as it is only reading.



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

Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Change-Id: If082703cf81a436ae5a565c75225fa8c0554b702
Gerrit-Change-Number: 16222
Gerrit-PatchSet: 5
Gerrit-Owner: N A <mail at interlinked.x10host.com>
Gerrit-Reviewer: Friendly Automation
Gerrit-Reviewer: Joshua Colp <jcolp at sangoma.com>
Gerrit-Attention: N A <mail at interlinked.x10host.com>
Gerrit-Comment-Date: Thu, 05 Aug 2021 11:59:42 +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/20210805/b69bc980/attachment.html>