Attention is currently required from: wbokslag.
Patch set 3:Code-Review -1
3 comments:
File src/tetra_upper_mac.h:
sounds like 'active' could be of type bool?
would be nice to have some comments explaining the contents for the non-obvious ones such as 'age' and 'encrypytion'.
The age comment should not just explain the units, but also explain the rationale of having a signed type for the age.
For 'encryption' it also would be good to explain what it is for. Is it just a flag? or an indication of a specific algorithm?... In the code I also see stuff like 'encryption = encryption_mode' where the latter is an unsigned type. So once again the question arises why a signed type is used to represent something that's unsigned.
File src/tetra_upper_mac.c:
this introduces a global, static variable. Normally all state should be contained witin the tetra_mac_state or whatever other related 'context' structure to ensure the code works also if there are many instances in parallel, without any clashes over global variables storing state that logically belongs to an instance.
Patch Set #3, Line 50: fragslots[i].msgb = msgb_alloc(FRAGSLOT_MSGB_SIZE, "fragslot");
is there a specific reason to pre-allocate lots of message buffers here?
To view, visit change 29387. To unsubscribe, or for help writing mail filters, visit settings.