Attention is currently required from: laforge, pespin, fixeria. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-msc/+/30114 ) Change subject: add codec_mapping.h,c ...................................................................... Patch Set 4: (3 comments) File include/osmocom/msc/codec_mapping.h: https://gerrit.osmocom.org/c/osmo-msc/+/30114/comment/3485efdb_5dd973b7 PS4, Line 44: for ((CODEC_MAPPING) = codec_map; (CODEC_MAPPING) < codec_map + ARRAY_SIZE(codec_map); (CODEC_MAPPING)++) TL;dr: I'll move this to the .c file and don't even publish the array in the .h file. The mapping entries are always accessed via functions. details: so we need to solve the fundamental question whether we can trust our ARRAY_SIZE() macro. It's not the preprocessor that needs to know, the preproc resolves to sizeof(this)/sizeof(that), and then the compiler figures that out compile time. if sizeof() didn't work on the thing passed to it, i.e. if the storage size were unknown, the compiler would tell us. To illustrate, i added this code to some random place in osmo-msc: printf("hi %zu\n", ARRAY_SIZE(osmo_tdef_unit_names)); it calls ARRAY_SIZE on a libosmocore "unsized" array. then we get: error: invalid application of ‘sizeof’ to incomplete type ‘const struct value_string[]’ 54 | printf("hi %zu\n", ARRAY_SIZE(osmo_tdef_unit_names)); | ^~~~~~~~~~ So the main question is whether this codec mapping should be movable to a different library -- This array is used only within osmo-msc, actually only accessed directly from codec_mapping.c, so it doesn't need to be in the .h file at all File src/libmsc/codec_mapping.c: https://gerrit.osmocom.org/c/osmo-msc/+/30114/comment/821d024f_68f15f83 PS4, Line 290: if (bearer_cap->speech_ver[i] == -1) { seech_ver = -1 is used all over the GSM code; we can change that but that's orthogonal https://gerrit.osmocom.org/c/osmo-msc/+/30114/comment/4c27956e_188b1a5f PS4, Line 308: if (bearer_cap->speech_ver[i] == -1) Done -- To view, visit https://gerrit.osmocom.org/c/osmo-msc/+/30114 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-msc Gerrit-Branch: master Gerrit-Change-Id: Iaa307be6a8487aa8d4ba7cd59d5c5ef04818a744 Gerrit-Change-Number: 30114 Gerrit-PatchSet: 4 Gerrit-Owner: neels &lt;nhofmeyr(a)sysmocom.de&gt; Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge &lt;laforge(a)osmocom.org&gt; Gerrit-Reviewer: pespin &lt;pespin(a)sysmocom.de&gt; Gerrit-CC: fixeria &lt;vyanitskiy(a)sysmocom.de&gt; Gerrit-Attention: laforge &lt;laforge(a)osmocom.org&gt; Gerrit-Attention: pespin &lt;pespin(a)sysmocom.de&gt; Gerrit-Attention: fixeria &lt;vyanitskiy(a)sysmocom.de&gt; Gerrit-Comment-Date: Tue, 07 Mar 2023 15:38:25 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: laforge &lt;laforge(a)osmocom.org&gt; Comment-In-Reply-To: pespin &lt;pespin(a)sysmocom.de&gt; Gerrit-MessageType: comment