Attention is currently required from: neels, pespin, dexter. fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/29475 )
Change subject: fix gsm0808_sc_cfg <-> gsm48_mr_cfg conversion ...................................................................... Patch Set 4: (1 comment) File include/osmocom/gsm/protocol/gsm_04_08.h: https://gerrit.osmocom.org/c/libosmocore/+/29475/comment/65ec494f_715300db PS2, Line 627: return ((uint8_t *)cfg)[1]; > gsm48_multi_rate_conf has two uint8_t members, AFAICT adding a union would > require adding another ex […] You don't really need to name the union itself, it can be anonymous (just like structures). The problem is that bit-fields are treated as individual fields in a union, so doing what Pau suggested: ``` /* I removed the LE variant for the sake of readability * see https://people.osmocom.org/fixeria/union_bfield_test.c */ struct gsm48_multi_rate_conf { uint8_t ver:3, nscb:1, icmi:1, spare:1, smod:2; union { uint8_t m12_2:1, m10_2:1, m7_95:1, m7_40:1, m6_70:1, m5_90:1, m5_15:1, m4_75:1; uint8_t mode; }; } __attribute__((packed)); ``` will result in the following behavior: ``` struct gsm48_multi_rate_conf mrc = { 0 }; // mrc.mode = 0x00 // m4_75=0, m5_15=0, m5_90=0, m6_70=0, m7_40=0, m7_95=0, m10_2=0, m12_2=0 mrc.m4_75 = 1; // mrc.mode = 0x01 // m4_75=1, m5_15=1, m5_90=1, m6_70=1, m7_40=1, m7_95=1, m10_2=1, m12_2=1 mrc.m10_2 = 0; // mrc.mode = 0x00 // m4_75=0, m5_15=0, m5_90=0, m6_70=0, m7_40=0, m7_95=0, m10_2=0, m12_2=0 mrc.m12_2 = 1; // mrc.mode = 0x01 // m4_75=1, m5_15=1, m5_90=1, m6_70=1, m7_40=1, m7_95=1, m10_2=1, m12_2=1 ``` Definitely not what we want. Here is an alternative approach: ``` union gsm48_multi_rate_conf_u { struct gsm48_multi_rate_conf mrc; struct { uint8_t hdr; uint8_t mode; }; }; ``` This should work as expected, but I am fine with the current approach. Keep it as it is. -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/29475 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I900fda192742fa8f6dd54e9131ef1704b14cc41a Gerrit-Change-Number: 29475 Gerrit-PatchSet: 4 Gerrit-Owner: neels <nhofm...@sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: dexter <pma...@sysmocom.de> Gerrit-Reviewer: laforge <lafo...@osmocom.org> Gerrit-CC: fixeria <vyanits...@sysmocom.de> Gerrit-CC: pespin <pes...@sysmocom.de> Gerrit-Attention: neels <nhofm...@sysmocom.de> Gerrit-Attention: pespin <pes...@sysmocom.de> Gerrit-Attention: dexter <pma...@sysmocom.de> Gerrit-Comment-Date: Wed, 01 Mar 2023 11:29:22 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: neels <nhofm...@sysmocom.de> Comment-In-Reply-To: pespin <pes...@sysmocom.de> Gerrit-MessageType: comment
