Attention is currently required from: laforge, pespin.
3 comments:
File src/codec/gsm660.c:
Patch Set #1, Line 271: static con
If you asked me my bet would be that a local non-static variable would always be allocated in the st […]
@laforge: I checked the code produced by gcc (5.3.0, came with Slackware 14.2) with and without the static, and if I drop the static, the compiler generates silly code: the actual table is still in .rodata, but it isn't entered into the symbol table as a sid_code_word_bits local symbol, instead there is a stack allocation (the actual "object") and the compiler emits code (basically an inline memcpy equivalent) that copies the table from .rodata to the stack. Putting the static back in results in sane generated code: just the table in .rodata, with the sid_code_word_bits local symbol on it, no extra stack allocation or copying. Hence I argue for keeping the static.
@pespin: you correctly interpreted my intent here.
/* bit numbers relative to "pure" EFR frame beginning,
* not counting the signature bits. */
45, 46, 48, 49, 50, 51, 52, 53, 54, 55,
56, 57, 58, 59, 60, 61, 62, 63, 64, 65,
66, 67, 68, 94, 95, 96, 98, 99, 100, 101,
102, 103, 104, 105, 106, 107, 108, 109, 110, 111,
112, 113, 114, 115, 116, 117, 118, 148, 149, 150,
151, 152, 153, 154, 155, 156, 157, 158, 159, 160,
161, 162, 163, 164, 165, 166, 167, 168, 169, 170,
171, 196, 197, 198, 199, 200, 201, 202, 203, 204,
205, 206, 207, 208, 209, 212, 213, 214, 215, 216,
217, 218, 219, 220, 221 };
Ack
I was going for the same code structure and style as in osmo_fr_check_sid() in gsm610.c, but I agree that the code looks better and more stylistically correct with one less tab of indentation. I'll be spinning the next patch version with the reduced indent.
Patch Set #1, Line 293: for (i = 0; i < ARRAY_SIZE(sid_code_word_bits); i++)
In general we always use {} in code blocks with multiple lines in it.
Once again I was copying the code structure and style from osmo_fr_check_sid() and osmo_hr_check_sid(), where the same construct appears without the braces. But following the principle that newly added code should be styled "right", I am changing the construct in question to the braced style in the next patch version.
To view, visit change 32034. To unsubscribe, or for help writing mail filters, visit settings.