On 11/01/2023 22:26, Vadim Yanitskiy wrote:
Hi Vadim, just to say, I'm preparing a patch for this... see below:
I guess the limit of 5 octets is defined by the specs? If so, then
it's better to sacrifice the most significant octet (usually 0x80 for
CC) of the CallRef, rather than the least significant one.
== Option a):
| 0 | 1 | 2 | 3 | 4 |
^^^^^^^^^^^ CallRef at &lcls->gcr.cr[0]
^^^^^^^ LAC/CID at &lcls->gcr.cr[3]
lcls->gcr.cr[2] = (trans->callref >> 0) & 0xff;
lcls->gcr.cr[1] = (trans->callref >> 8) & 0xff;
lcls->gcr.cr[0] = (trans->callref >> 16) & 0xff;
osmo_store16be(/* LAC/CID */, &lcls->gcr.cr[3]);
== Option b):
| 0 | 1 | 2 | 3 | 4 |
^^^^^^^^^^^ CallRef at &lcls->gcr.cr[2]
^^^^^^^ LAC/CID at &lcls->gcr.cr[0]
/* NOTE: LAC/CID overwrites MSB of CallRef at &lcls->gcr.cr[1] */
osmo_store32be(/* CallRef */, &lcls->gcr.cr[1]);
osmo_store16be(/* LAC/CID */, &lcls->gcr.cr[0]);
Personally I would prefer option b), because hexadecimal
representation of the resulting lcls->gcr.cr[] would look similar to
the CallRef in hex, so it would be easier to match them in logging.
The spec actually forces us to option A, unfortunately. I don't really
know how much it matters for us, or for anything else, that we might
break it. Anyway, see you in Code Review?
Many thanks,
Keith.
