<p><a href="https://gerrit.asterisk.org/c/asterisk/+/14887">View Change</a></p><p>3 comments:</p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.asterisk.org/c/asterisk/+/14887/5/include/asterisk/logger_category.h">File include/asterisk/logger_category.h:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/asterisk/+/14887/5/include/asterisk/logger_category.h@97">Patch Set #5, Line 97:</a> <code style="font-family:monospace,monospace">const char *name</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">Should this be the ID? Not sure.</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">"name" is correct, as it does the look up by name. One place it is called from is "ast_debug_category_set_sublevels" which takes a list of names and passes the individual names to this function.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/asterisk/+/14887/5/include/asterisk/logger_category.h@119">Patch Set #5, Line 119:</a> <code style="font-family:monospace,monospace">const char * const *names, size_t size</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">How about making this a ast_vector_const_string? It'd be easier to work with.</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">The reason I didn't do it that way is because this function is currently only called from the "core set debug categories ..." CLI command, which uses an array of strings. And it abstracts iterating over that array, so if say in the future verbose categories were added this function could be reused.</p><p style="white-space: pre-wrap; word-wrap: break-word;">If this took a vector much of the functionality here would then need to be pushed up into the CLI command function. The array would be iterated and parsed there, strings copied, and then the vector iterated again here.</p><p style="white-space: pre-wrap; word-wrap: break-word;">A vector version could be added in the future if something else ends up needing similar functionality but it was easier to use a vector in that case.</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.asterisk.org/c/asterisk/+/14887/5/main/logger_category.c">File main/logger_category.c:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/asterisk/+/14887/5/main/logger_category.c@42">Patch Set #5, Line 42:</a> </p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;"><pre style="font-family: monospace,monospace; white-space: pre-wrap;">static uintmax_t get_next_id(struct categories_level_t *level)<br>{<br> if (level->id_pool == 0) {<br> level->id_pool = 1;<br> } else if (level->id_pool >= (UINTMAX_MAX / 2)) {<br> /* No more ids left*/<br> return 0;<br> } else {<br> level->id_pool <<= 1;<br> }<br><br> return level->id_pool;<br>}<br><br>static int cmp_by_name(const struct category_t *category, const char *name)<br>{<br> return !strcmp(category->name, name);<br>}<br><br>static uintmax_t category_register(struct categories_level_t *level, const char *name)<br>{<br> int i;<br> struct category_t *category;<br><br> AST_VECTOR_RW_WRLOCK(&level->categories);<br><br> i = AST_VECTOR_GET_INDEX(&level->categories, name, cmp_by_name);<br> if (i >= 0) {<br> AST_VECTOR_RW_UNLOCK(&level->categories);<br> ast_log(LOG_ERROR, "Cannot register logger category '%s'. "<br> "Name already used for type.\n", name);<br> return 0;<br> }<br><br> category = ast_calloc(1, sizeof(*category) + strlen(name) + 1);<br> if (!category) {<br> AST_VECTOR_RW_UNLOCK(&level->categories);<br> return 0;<br> }<br><br> category->id = get_next_id(level);<br> category->sublevel = AST_LOG_CATEGORY_DISABLED;<br> strcpy(category->name, name); /* Safe */<br><br> if (AST_VECTOR_APPEND(&level->categories, category)) {<br> AST_VECTOR_RW_UNLOCK(&level->categories);<br> ast_log(LOG_ERROR, "Cannot register logger category '%s'. "<br> "Unable to append.\n", name);<br> return 0;<br> }<br><br> AST_VECTOR_RW_UNLOCK(&level->categories);<br> ast_log(LOG_VERBOSE, "##### %s - %ju\n", category->name, category->id);<br> return category->id;<br>}<br><br></pre></blockquote></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">I'm a little confused. […]</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Ids must be a power of 2. This allows for fast lookup, and "or'ing" of ids in order to permit multiple categories in a log statement.</p><p style="white-space: pre-wrap; word-wrap: break-word;">Using a vector would require a for loop on every category log statement called. It would also make specifying multiple categories for a given log statement a little awkward.</p><p style="white-space: pre-wrap; word-wrap: break-word;">I'll add a comment in the code.</p></li></ul></li></ul><p>To view, visit <a href="https://gerrit.asterisk.org/c/asterisk/+/14887">change 14887</a>. To unsubscribe, or for help writing mail filters, visit <a href="https://gerrit.asterisk.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.asterisk.org/c/asterisk/+/14887"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: asterisk </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-Change-Id: I6e6cb247bb1f01dbf34750b2cd98e5b5b41a1849 </div>
<div style="display:none"> Gerrit-Change-Number: 14887 </div>
<div style="display:none"> Gerrit-PatchSet: 5 </div>
<div style="display:none"> Gerrit-Owner: Kevin Harwell <kharwell@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: Friendly Automation </div>
<div style="display:none"> Gerrit-Reviewer: George Joseph <gjoseph@digium.com> </div>
<div style="display:none"> Gerrit-Comment-Date: Tue, 15 Sep 2020 15:33:29 +0000 </div>
<div style="display:none"> Gerrit-HasComments: Yes </div>
<div style="display:none"> Gerrit-Has-Labels: No </div>
<div style="display:none"> Comment-In-Reply-To: George Joseph <gjoseph@digium.com> </div>
<div style="display:none"> Gerrit-MessageType: comment </div>