<p><a href="https://gerrit.osmocom.org/13121">View Change</a></p><p>6 comments:</p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.osmocom.org/#/c/13121/2/include/osmocom/core/use_count.h">File include/osmocom/core/use_count.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.osmocom.org/#/c/13121/2/include/osmocom/core/use_count.h@66">Patch Set #2, Line 66:</a> <code style="font-family:monospace,monospace">* osmo_use_count_cb_t </code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">Never seen such spacing style before.</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">it's my personal favorite way to space function pointer typedefs. I find the "(*name)" really weird as language design, so I like to set it apart from the name. Especially if a pointer is returned...</p><pre style="font-family: monospace,monospace; white-space: pre-wrap;">  typedef foo *(*bar)(args);</pre><p style="white-space: pre-wrap; word-wrap: break-word;">always confuses me... for me it helps to write</p><pre style="font-family: monospace,monospace; white-space: pre-wrap;">  foo* (* bar )(args);</pre><p style="white-space: pre-wrap; word-wrap: break-word;">so that bar doesn't look like function arguments or a typecast...</p><p style="white-space: pre-wrap; word-wrap: break-word;">Can change if you insist?</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/#/c/13121/2/include/osmocom/core/use_count.h@141">Patch Set #2, Line 141:</a> <code style="font-family:monospace,monospace"> */</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">Also looks a bit weird...</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">that's your standard linux torvalds paper scroll way...</p><p style="white-space: pre-wrap; word-wrap: break-word;">ok the standard way would be</p><pre style="font-family: monospace,monospace; white-space: pre-wrap;">  /*<br>   * words<br>   */</pre><p style="white-space: pre-wrap; word-wrap: break-word;">but the known kernel code argument is that this is "braindamaged":</p><pre style="font-family: monospace,monospace; white-space: pre-wrap;">  /* words<br>   * words */</pre></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/#/c/13121/2/include/osmocom/core/use_count.h@150">Patch Set #2, Line 150:</a> <code style="font-family:monospace,monospace">count</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">I'm not arguing agains the requirement / use case. […]</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">ah ok, a naming issue, I understand now. will ponder it...</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.osmocom.org/#/c/13121/2/tests/use_count/use_count_test.c">File tests/use_count/use_count_test.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/13121/2/tests/use_count/use_count_test.c@320">Patch Set #2, Line 320:</a> <code style="font-family:monospace,monospace">"use_count_test.c"</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">Why not __FILE__?</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">because it contains the full relative path and depends on where configure was launched from. we could go for 'strrchr(__FILE__, '/') + 1' but that is obscure, unsafe and actually more characters.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/#/c/13121/2/tests/use_count/use_count_test.c@327">Patch Set #2, Line 327:</a> <code style="font-family:monospace,monospace">     log_set_print_filename2(osmo_stderr_target, LOG_FILENAME_NONE);</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">wait a minute, this looks redundant</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/#/c/13121/2/tests/use_count/use_count_test.c@326">Patch Set #2, Line 326:</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;">log_set_print_filename2(osmo_stderr_target, LOG_FILENAME_BASENAME);<br>      log_set_print_filename2(osmo_stderr_target, LOG_FILENAME_NONE);<br>       log_set_print_filename_pos(osmo_stderr_target, LOG_FILENAME_POS_LINE_END);<br>    log_set_print_category(osmo_stderr_target, 1);<br>        log_set_print_category_hex(osmo_stderr_target, 0);<br>    log_set_print_level(osmo_stderr_target, 1);<br>   log_set_use_color(osmo_stderr_target, 0);<br></pre></blockquote></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">Off-topic: I think we need some generic configuration for all unit tests.</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">most of them don't care about logging...</p></li></ul></li></ul><p>To view, visit <a href="https://gerrit.osmocom.org/13121">change 13121</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/13121"/><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-MessageType: comment </div>
<div style="display:none"> Gerrit-Change-Id: Ife31e6798b4e728a23913179e346552a7dd338c0 </div>
<div style="display:none"> Gerrit-Change-Number: 13121 </div>
<div style="display:none"> Gerrit-PatchSet: 2 </div>
<div style="display:none"> Gerrit-Owner: Neels Hofmeyr <nhofmeyr@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: Harald Welte <laforge@gnumonks.org> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins Builder (1000002) </div>
<div style="display:none"> Gerrit-Reviewer: Neels Hofmeyr <nhofmeyr@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: Vadim Yanitskiy <axilirator@gmail.com> </div>
<div style="display:none"> Gerrit-Comment-Date: Mon, 11 Mar 2019 05:55:06 +0000 </div>
<div style="display:none"> Gerrit-HasComments: Yes </div>
<div style="display:none"> Gerrit-HasLabels: No </div>