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 <pes...@sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanits...@sysmocom.de> Gerrit-Reviewer: laforge <lafo...@osmocom.org> Gerrit-Attention: laforge <lafo...@osmocom.org> Gerrit-Attention: pespin <pes...@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 <pes...@sysmocom.de> Comment-In-Reply-To: fixeria <vyanits...@sysmocom.de> Gerrit-MessageType: comment
