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/.
pespin gerrit-no-reply at lists.osmocom.orgpespin 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 {
> i used to be a big fan of opaque structs, but every time i used them it became a big cumbersome api bloat
Well it depends on the case. for the connection structs in libosmo-netif, having opaque structs proved to be a good way. We could extend functionalities several times without breaking ABI compatiblity.
> Derefernces of osmo_stat_item: osmo_stat_item_handler() in stats.c uses item->desc
Code inside libosmocore can still access members directly, that's not a poroblem, that's why I was mentioning moving the structs to an internal header instead of the .c file.
> Callers outside of libosmocore [...] appears though that no osmocom code outside of libosmocore is doing so
I don't expect code outside of libosmocore to access those, that's why I'm mentioning about making a opaque struct. If somebody anypoint in time needs more access, APIs can be added.
I also had to break the ABI lately around these structs, and had to add an API to avoid breaking it in the future. That's why I'm saying t looks like a good moment to make the structs opaque.
I'm not blocking the merge of this patch as it is, I just want to raise my concerns in order to avoid similar issues in the future.
--
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 10:53:25 +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/b94f234a/attachment.htm>