Attention is currently required from: laforge, pespin.
fixeria has posted comments on this change. (
https://gerrit.osmocom.org/c/libosmocore/+/32014 )
Change subject: Fix parsing of TLV_TYPE_SINGLE_TV
......................................................................
Patch Set 1:
(1 comment)
File include/osmocom/gsm/protocol/gsm_04_08.h:
https://gerrit.osmocom.org/c/libosmocore/+/32014/comment/8d0c93de_c9d67ffb
PS1, Line 1779: GSM48_IE_CIP_MODE_SET
1- Stuff is already broken. It's not possible to
encode and decode those IEs properly in a sane way.
Let's clarify: the libosmocore's TLV codec is handling `TLV_TYPE_SINGLE_TV`
incorrectly, yes. But not all projects are using it, osmo-bsc.git does the encoding of
some PDUs manually (by using the msgb API), and it's doing it correctly. So not
everything is broken.
2- That what libversion is for. We make them depend on
newer libversion in master and next releases.
Not sure if I am following you. AFAIU, you're talking about fixing osmo-bsc.git and
making it depend on more recent fixed libosmocore. But I am talking about compiling old
osmo-bsc (e.g. while bisecting) against recent (to be fixed) libosmocore. If you change
define values, old code doing manual encoding of these IEIs will start producing wrong
encoding results. To me it's even worse than breaking the API and breaking compilation
for old code, because it will cause problems at run-time.
IMO, we should rather keep the old GSM48_IE_ for backwards compatibility with projects
using them like osmo-bsc does, define new ones with the correct values and use these new
ones in `gsm48_rr_att_tlvdef[]`.
I know it's a pain changing those and breaking the
ABI, but otherwise we are just mtairning a broken system and keep adding workaround
everywhere.
Well, yes, it's a pain. But I don't remember we ever changed our policy about
keeping backwards compatibility and started breaking the API here and there? It's
similar to deprecating functions: just add a comment marking old defines as deprecated,
add new ones with the correct values, and use these new ones everywhere.
--
To view, visit
https://gerrit.osmocom.org/c/libosmocore/+/32014
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I799e35dc8d4d153fa63bf50563a5482cdf4de2d7
Gerrit-Change-Number: 32014
Gerrit-PatchSet: 1
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Wed, 22 Mar 2023 13:00:03 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>
Comment-In-Reply-To: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-MessageType: comment