<p> Attention is currently required from: N A, Joshua Colp. </p>
<p><a href="https://gerrit.asterisk.org/c/asterisk/+/16222">View Change</a></p><p>2 comments:</p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0;"><p><a href="null">File main/logger.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/+/16222/comment/57aa53da_aab9bae5">Patch Set #12, Line 847:</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;"> for (level = 16; level < ARRAY_LEN(levels); level++) {<br> if (levels[level] && custom_dynamic_levels[level]) {<br> int level2, found = 0;<br> for (level2 = 16; level2 < ARRAY_LEN(levels); level2++) {<br> if (levels[level2] && !strcasecmp(levels[level2], levels[level])) {<br> found = 1; /* level still exists, skip */<br> break;<br> }<br> }<br></pre></blockquote></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">Not quite sure I follow... can you elaborate on this? […]</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Your new patch is better, but I'll follow with a comment there and talk about what I meant.</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="null">File main/logger.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/+/16222/comment/bd112660_ab8b89e7">Patch Set #14, Line 832:</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;"> for (level = 16; level < ARRAY_LEN(levels); level++) {<br> if (levels[level] && custom_dynamic_levels[level]) {<br> logger_unregister_level(levels[level]);<br> custom_dynamic_levels[level] = 0;<br> }<br> }<br></pre></blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">If I understand this bit of code correctly then all currently registered custom dynamic levels get unregistered, which then they possibly get re-registered again below. This might be fine to do, but</p><p style="white-space: pre-wrap; word-wrap: break-word;">1) Would it be possible for a thread to get a level, the level changes for that custom value, and then write to previously retrieved level? Thus potentially writing to the wrong level.</p><p style="white-space: pre-wrap; word-wrap: break-word;">2) Similar to the above, but no level may be available for a short time. Meaning could all custom levels be removed, and then another thread ask for one and get "no matching custom level available"? I think this second case is protected by current locks though?</p><p style="white-space: pre-wrap; word-wrap: break-word;">Anyway if either of those are problems, or you just wanted to do it differently such that you only removed custom levels that are no longer configured then I was thinking of something like the following (continuation of a previous comment):</p><p style="white-space: pre-wrap; word-wrap: break-word;">Instead of looping over the level array here, you'd want to loop over the custom_dynamic_levels array, and see if it matched with any in the incoming "customlogs" (a 'strstr' call might be sufficient?). If so do nothing/keep it. If no match then remove from custome_dynamic_levels array and unregister from levels.</p></li></ul></li></ul><p>To view, visit <a href="https://gerrit.asterisk.org/c/asterisk/+/16222">change 16222</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/+/16222"/><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: If082703cf81a436ae5a565c75225fa8c0554b702 </div>
<div style="display:none"> Gerrit-Change-Number: 16222 </div>
<div style="display:none"> Gerrit-PatchSet: 14 </div>
<div style="display:none"> Gerrit-Owner: N A <mail@interlinked.x10host.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-Reviewer: Joshua Colp <jcolp@sangoma.com> </div>
<div style="display:none"> Gerrit-Reviewer: Kevin Harwell <kharwell@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: N A <mail@interlinked.x10host.com> </div>
<div style="display:none"> Gerrit-Attention: N A <mail@interlinked.x10host.com> </div>
<div style="display:none"> Gerrit-Attention: Joshua Colp <jcolp@sangoma.com> </div>
<div style="display:none"> Gerrit-Comment-Date: Wed, 15 Sep 2021 16:12:25 +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: N A <mail@interlinked.x10host.com> </div>
<div style="display:none"> Comment-In-Reply-To: Kevin Harwell <kharwell@digium.com> </div>
<div style="display:none"> Gerrit-MessageType: comment </div>