Attention is currently required from: laforge, pespin, fixeria.

3 comments:

File include/osmocom/msc/codec_mapping.h:
- Patch Set #4, Line 44: for ((CODEC_MAPPING) = codec_map; (CODEC_MAPPING) < codec_map + ARRAY_SIZE(codec_map); (CODEC_MAPPING)++)
  It's an interesting question if the preprocessor really can determine the size here or not. […]
  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:
- Patch Set #4, Line 290: if (bearer_cap->speech_ver[i] == -1) {
  I actually think -1 is more obvious, and I think we have plenty of similar examples
  seech_ver = -1 is used all over the GSM code; we can change that but that's orthogonal
- Patch Set #4, Line 308: if (bearer_cap->speech_ver[i] == -1)
  -1 as define
  Done

To view, visit change 30114. To unsubscribe, or for help writing mail filters, visit settings.