Attention is currently required from: fixeria, jolly, laforge, pespin.
2 comments:
File include/osmocom/netif/twjit.h:
Patch Set #6, Line 50: struct osmo_twjit_config {
The only thing left which I'm now concerned is regarding exposing all these structs here, which mean […]
The only struct that poses truly serious ABI issues is `osmo_twjit_config`. In ThemWi applications using `twrtp-native`, and also in my on-branches OsmoBTS patches prior to you raising this issue just now, the twjit config struct is embedded directly into some application data structure. This arrangement is not a problem with `twrtp-native` as the latter is a static lib, but I see now how it would be a problem in Osmocom-integrated version. If an application is compiled against one version of `libosmo-netif` and its header files, and then the shared lib is replaced with a newer version in which twjit config struct gained extra fields, breakage will ensue: created/configured twjit instances will read garbage in the new fields, while vty code in the library will write past the end of the application-provided struct.
The only solution I can think of is to replace `osmo_twjit_config_init()` with `osmo_twjit_config_create()` or `osmo_twjit_config_alloc()` (please indicate which naming you prefer), make this config struct fully opaque, and implement setter functions for those rare apps that might need to control these settings through non-vty means.
However, in the case of `osmo_twjit_stats`, `osmo_twjit_rr_info` and `osmo_twrtp_stats` addressed in the other patch, I argue we can go for a simpler approach. All of these structures are only allocated by the library inside opaque twjit and twrtp instance structs, while library users get only const pointers to them. Therefore, ABI breakage can be avoided by adopting a policy inside the library (documented with inline comments) that these structs can be extended only by adding new fields at the end. Already defined fields cannot be dropped - just the API alone imposes this requirement, before considering ABI - but for ABI reasons, they cannot be reordered either.
I shall wait for your feedback before I implement this proposed solution.
File src/twjit.c:
Patch Set #2, Line 504: rtph = osmo_rtp_get_hdr(msg);
The M bit feature was implemented in Patchset 6 as you asked. Please provide feedback.
Can this issue be marked as resolved - is M bit handled to your satisfaction now?
To view, visit change 39280. To unsubscribe, or for help writing mail filters, visit settings.