<p><a href="https://gerrit.osmocom.org/c/libosmocore/+/25464">View Change</a></p><p>1 comment:</p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.osmocom.org/c/libosmocore/+/25464/5/include/osmocom/core/stat_item.h">File include/osmocom/core/stat_item.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/libosmocore/+/25464/5/include/osmocom/core/stat_item.h@17">Patch Set #5, Line 17:</a> <code style="font-family:monospace,monospace">struct osmo_stat_item_period {</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">Yeah, but does "struct osmo_stat_item" itself need to be public? probably not.</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">i used to be a big fan of opaque structs, but every time i used them it became a big cumbersome api bloat.</p><p style="white-space: pre-wrap; word-wrap: break-word;">we are changing pre-existing API. i guess the dam has broken and we might as well change *everything* now. Derefernces of osmo_stat_item: osmo_stat_item_handler() in stats.c uses item->desc. Callers outside of libosmocore may also access item->desc; it appears though that no osmocom code outside of libosmocore is doing so. so kind of yes it could be made opaque, while we're already breaking API. what's the benefit? no future api breakage if the implementation changes again. what's the drawback? need to introduce api functions for reading stat numbers, if anyone needs them; from the current API breakage we know that every osmocom code that does access the current stat number is already using osmo_stat_item_get_last(). I guess pespin has a point there.</p><p style="white-space: pre-wrap; word-wrap: break-word;">i have a strong feeling that i'd love to get out of the bikeshed phase some time soon. we are still getting those DLSTATS error messages all the while, until this patch is merged. so i am tempted to just merge it now... but considering opaqueness.</p></li></ul></li></ul><p>To view, visit <a href="https://gerrit.osmocom.org/c/libosmocore/+/25464">change 25464</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/+/25464"/><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: I137992a5479fc39bbceb6c6c2af9c227bd33b39b </div>
<div style="display:none"> Gerrit-Change-Number: 25464 </div>
<div style="display:none"> Gerrit-PatchSet: 5 </div>
<div style="display:none"> Gerrit-Owner: neels <nhofmeyr@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins Builder </div>
<div style="display:none"> Gerrit-Reviewer: daniel <dwillmann@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: laforge <laforge@osmocom.org> </div>
<div style="display:none"> Gerrit-CC: fixeria <vyanitskiy@sysmocom.de> </div>
<div style="display:none"> Gerrit-CC: osmith <osmith@sysmocom.de> </div>
<div style="display:none"> Gerrit-CC: pespin <pespin@sysmocom.de> </div>
<div style="display:none"> Gerrit-Comment-Date: Wed, 29 Sep 2021 09:40:50 +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: neels <nhofmeyr@sysmocom.de> </div>
<div style="display:none"> Comment-In-Reply-To: pespin <pespin@sysmocom.de> </div>
<div style="display:none"> Comment-In-Reply-To: daniel <dwillmann@sysmocom.de> </div>
<div style="display:none"> Gerrit-MessageType: comment </div>