[PATCH] Addditional classmark check against A5/X support

Sylvain Munaut 246tnt at gmail.com
Tue Jun 17 09:15:31 UTC 2014


On Tue, Jun 17, 2014 at 10:56 AM, ☎ <Max.Suraev at fairwaves.co> wrote:
> 16.06.2014 23:04, Sylvain Munaut пишет:
>>
>>> -                       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 ...
>>
>
> 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.



>>> +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.
>>
>
> 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




More information about the baseband-devel mailing list