<p>Patch set 3:<span style="border-radius: 3px; display: inline-block; margin: 0 2px; padding: 4px;background-color: #ffd4d4;">Code-Review -1</span></p><p><a href="https://gerrit.osmocom.org/c/libosmocore/+/19442">View Change</a></p><p>11 comments:</p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.osmocom.org/c/libosmocore/+/19442/3/src/vty/sched_vty.c">File src/vty/sched_vty.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.osmocom.org/c/libosmocore/+/19442/3/src/vty/sched_vty.c@279">Patch Set #3, Line 279:</a> <code style="font-family:monospace,monospace">"Set CPU affinity mask of the thread\n"</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">This description only applies to 'self', while the rest ('all', 'THREADNAME', '<0-4294967295>') is not documented. Each '(x|y|z)' entry needs a separate doc-string. Please make sure to add the VTY node tests (see tdef/tdef_vty_test_dynamic.vty, for example).</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/libosmocore/+/19442/3/src/vty/sched_vty.c@315">Patch Set #3, Line 315:</a> <code style="font-family:monospace,monospace">parsing</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">Maybe s/parsing/to parse/?</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/libosmocore/+/19442/3/src/vty/sched_vty.c@323">Patch Set #3, Line 323:</a> <code style="font-family:monospace,monospace">setting</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">Same here.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/libosmocore/+/19442/3/src/vty/sched_vty.c@361">Patch Set #3, Line 361:</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;">memset(&param, 0, sizeof(param));<br>        param.sched_priority = prio;<br></pre></blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Cosmetic: could be ... param = { .prio = 0 };</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/libosmocore/+/19442/3/src/vty/sched_vty.c@377">Patch Set #3, Line 377:</a> <code style="font-family:monospace,monospace">Use</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">Set?</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/libosmocore/+/19442/3/src/vty/sched_vty.c@378">Patch Set #3, Line 378:</a> <code style="font-family:monospace,monospace">Real time priority</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">Something also looks wrong here: you have three patterns in "policy rr <1-32>", but there are four help messages?</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/libosmocore/+/19442/3/src/vty/sched_vty.c@430">Patch Set #3, Line 430:</a> <code style="font-family:monospace,monospace">if(</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">coding style</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/libosmocore/+/19442/3/src/vty/sched_vty.c@434">Patch Set #3, Line 434:</a> <code style="font-family:monospace,monospace">(long int)mypid, (long int) tid_it)</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">cosmetic: inconsistent spacing</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/libosmocore/+/19442/3/src/vty/sched_vty.c@452">Patch Set #3, Line 452:</a> <code style="font-family:monospace,monospace">(1 << cpu_i);</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">UBSan usually warns that this needs to be explicitly casted to typeof(mask), because AFAIU by default (1 << x) is an integer of the platform specific size.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/libosmocore/+/19442/3/src/vty/sched_vty.c@484">Patch Set #3, Line 484:</a> <code style="font-family:monospace,monospace">vty</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">Doxygen: 'vty' (no such parameter) vs 'tall_ctx'</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/libosmocore/+/19442/3/src/vty/sched_vty.c@487">Patch Set #3, Line 487:</a> <code style="font-family:monospace,monospace">int</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">void? You always return 0 anyway...</p></li></ul></li></ul><p>To view, visit <a href="https://gerrit.osmocom.org/c/libosmocore/+/19442">change 19442</a>. To unsubscribe, or for help writing mail filters, visit <a href="https://gerrit.osmocom.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.osmocom.org/c/libosmocore/+/19442"/><meta itemprop="name" content="View Change"/></div></div>

<div style="display:none"> Gerrit-Project: libosmocore </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-Change-Id: If76a4bd2cc7b3c7adf5d84790a944d78be70e10a </div>
<div style="display:none"> Gerrit-Change-Number: 19442 </div>
<div style="display:none"> Gerrit-PatchSet: 3 </div>
<div style="display:none"> Gerrit-Owner: pespin <pespin@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins Builder </div>
<div style="display:none"> Gerrit-Reviewer: fixeria <vyanitskiy@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: laforge <laforge@osmocom.org> </div>
<div style="display:none"> Gerrit-Comment-Date: Wed, 29 Jul 2020 22:02:03 +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>