<p style="white-space: pre-wrap; word-wrap: break-word;">After looking at this more, I'm wondering if the ids need to be part of a bitfield. If the id is the index into a vector then it's just as fast to check as if it were a bit in a uint64 and you don't have to deal with the trying to find an open bit. Once you register and get an id, the id should be used for all subsequent calls. That also spares you from having to search the vector by name.<br></p><p>Patch set 5:<span style="border-radius: 3px; display: inline-block; margin: 0 2px; padding: 4px;background-color: #ffd4d4; color: #000000;">Code-Review -1</span></p><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 style="white-space: pre-wrap; word-wrap: break-word;">Should this be the ID? Not sure.</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 style="white-space: pre-wrap; word-wrap: break-word;">How about making this a ast_vector_const_string? It'd be easier to work with.</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 style="white-space: pre-wrap; word-wrap: break-word;">I'm a little confused. Why not just return the slot in the vector as the id?</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: Thu, 10 Sep 2020 16:02:49 +0000 </div>
<div style="display:none"> Gerrit-HasComments: Yes </div>
<div style="display:none"> Gerrit-Has-Labels: Yes </div>
<div style="display:none"> Gerrit-MessageType: comment </div>