Change in libosmocore[master]: refactor stat_item: get rid of FIFO and "skipped" error

This is merely a historical archive of years 2008-2021, before the migration to mailman3.

A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/gerrit-log@lists.osmocom.org/.

neels gerrit-no-reply at lists.osmocom.org
Wed Sep 29 09:40:50 UTC 2021


neels has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/25464 )

Change subject: refactor stat_item: get rid of FIFO and "skipped" error
......................................................................


Patch Set 5:

(1 comment)

https://gerrit.osmocom.org/c/libosmocore/+/25464/5/include/osmocom/core/stat_item.h 
File include/osmocom/core/stat_item.h:

https://gerrit.osmocom.org/c/libosmocore/+/25464/5/include/osmocom/core/stat_item.h@17 
PS5, Line 17: struct osmo_stat_item_period {
> Yeah, but does "struct osmo_stat_item" itself need to be public? probably not.
i used to be a big fan of opaque structs, but every time i used them it became a big cumbersome api bloat.

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.

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.



-- 
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/25464
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings

Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I137992a5479fc39bbceb6c6c2af9c227bd33b39b
Gerrit-Change-Number: 25464
Gerrit-PatchSet: 5
Gerrit-Owner: neels <nhofmeyr at sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: daniel <dwillmann at sysmocom.de>
Gerrit-Reviewer: laforge <laforge at osmocom.org>
Gerrit-CC: fixeria <vyanitskiy at sysmocom.de>
Gerrit-CC: osmith <osmith at sysmocom.de>
Gerrit-CC: pespin <pespin at sysmocom.de>
Gerrit-Comment-Date: Wed, 29 Sep 2021 09:40:50 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: neels <nhofmeyr at sysmocom.de>
Comment-In-Reply-To: pespin <pespin at sysmocom.de>
Comment-In-Reply-To: daniel <dwillmann at sysmocom.de>
Gerrit-MessageType: comment
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20210929/cbd2fb0b/attachment.htm>


More information about the gerrit-log mailing list