[PATCH] Add A5 and GEA ciphers

[Sylvain Munaut]

Hi Max,

> Attached is A5/3,4 GEA3,4 implementation which was described today at OsmoDevCon.

Here's some comments on the first patch. (It's late and I'm tired,
I'll look at the rest later :p)


1) Why do you need to add stddef.h ?  Also before of spacing change,
you're removing an empty line that was there to separate the includes
from the function comment.

2) wrt to :

+uint16_t osmo_get2bytes(const uint8_t *a);
+void osmo_64pack2pbit(uint64_t in, pbit_t *out);

Theses essentially look like integer accessors, one is a read for
uint16_t in big endian and the other a store for uint64_t in little
endian, but same basic idea. But you're using completely different
naming schemes for both ... I think they should reflect that their
name should reflect the LE/BE part and the unaligned part as well, I
even think there are already macro somewhere for that.  I would also
add other formats like LE/BE 16/32/64 bits store/load all at once
rather than each format when needed. If we add accessort, might as
well add all the basic types. And finally those should really be
inline functions in the .h, no point of doing a function call for
that.

3) In include/osmocom/gsm/gsm_utils.h, for ms_a5n_support  you take
only one cm argument ... that means the app needs to know if it should
give cm3 or cm2, I would take cm2 and cm3 separately and the app just
has to give both. I might also extend ms_cm2_a5n_support and
ms_cm3_a5n_support to return -1 in case it couldn't be determined
(because the required test isn't in that classmark), but that last
point is just a suggestion.

4) TAB vs SPACE indentation !

5) I would split the patch further between CM functions / buffere
reversion / accessorts.

6) Is ROL16 really something we exepect to do at a lot of places ?
just asking ... in anycase, also should probably be inline.


Cheers,

    Sylvain