17 Jun
2014
17 Jun
'14
11:15 a.m.
On Tue, Jun 17, 2014 at 10:56 AM, ☎ <Max.Suraev(a)fairwaves.co> wrote:
I'm not sure what you mean in here - we call this function with some classmark
(either 2 or 3) and number from a5/n to check if this cipher support is indicated in
the classmark. Anyway - just a little convenience wrapper.
What I mean is that it's not "either 2 or 3" it _needs_ to be CM2 for
a5/[1-3] and CM3 for a5/[4-7] ... so that means that the caller _HAS_
to have the knowledge of which CM to check.
If you want it to make convenient, then I'd take _both_ CM2 and CM3 in
argument and choose the right one.
Possibly make it accept NULL for CM2/CM3 and if you need a missing
info to make the decision, return -EAGAIN.
And make proper doc for this behavior. Since this is a new function,
and this actually provides a useful info you can use error code for
this.
Cheers,
Sylvain
16.06.2014 23:04, Sylvain Munaut пишет:
I thought it's better to expose error case explicitly.
In this case you can't change the API. Because the case I showed above
is _exactly_ how it's used currently so if someone has a new
libosmocore and an old openbsc, it will build fine but not do the
right thing.
And in either case a return value of 0 for n>7 is certainly not wrong
since you can't possibly support an non-existent cipher.
- return 0;
+ return (n > 7) ? 0 : -1;
}
}
Why this change ? I mean, you'd now have to go over __every_ use of
that function in all projects and make sure it's not used in something
like :
if (!ms_cm2_a5n_support(cm2, n)) {
error
}
Because an invalid n is now going to return something != 0 ...
+static inline int ms_a5n_support(uint8_t *cm,
unsigned n) {
+ return ((n < 4) ? ms_cm2_a5n_support(cm, n) : ms_cm3_a5n_support(cm, n));
+}
+
Huh ... so the called has to know whether to give CM2 or CM3 ... you
might as well not have this method at all then and just require it to
call the right method.