Frederick Ros wrote:
[EMAIL PROTECTED] wrote :
| I think the check could be done with eu_check_symb_addr that was remove for
| incomming cmvs.
This function is crap... Would be better to rewrite it more clearly.
yes when I looked at it I change it to
static uint32_t eu_check_symb_addr ( char *saddr )
{
uint32_t cmv;
/*
* search the reference array for a match
*/
for(cmv = 0; cmv < NUM_CMVS; cmv++)
{
if(!memcmp(saddr[letter], symb_addresses[cmv], 4))
return 1;
}
return 0;
}
but I never finish the cleanning ..
> | I think the check could be done in kernelspace with
eu_check_symb_addr that was
> | remove for incomming cmvs.
>
> The drawback with the check in kernel space is that, if new functions
> are introduced (through a new BNM and new CMVs), then the driver need to
> be modified to take this new one into account...
> In fact there are pros and cons to even check the CMVs:
> - checking enforce that the CMVs are known
> - no check allow to add functions without changing anything beyond
> CMVs file and BNM files
>
> Moreover not checking is not a problem from the computer/kernel point of
> view, as the CMVs (or more precisely the function to call in the modem)
> are interpreted only by the modem ...
>
I think it could be a problem, if you could damage the modem by sending
it invalid command.
> | Also in eu_decode_msg the subid checking is a bit strange :
> | Something like
> | switch(msg->subtype) {
> | case SUBTYPE_FLASHACC_ENMODEMREBOOTREPLY ...
SUBTYPE_FLASHACC_READREPLY:
> | if (msg->type != MP_FUNCTION_TYPE_FLASHACC) {
> | error
> | }
> |
> | case ...
> |
> | would be a bit cleaner.
>
> I don't think it's clearer: we check the subtype, and say: in case we
> have the subtype x, and that the type was Y, do ...
> I do prefer the top to bottom approach ..
>
yes, I said that because most of subtype are only for a type, so it
avoid a big if in the case, but if the value are in the right order, the
if maybe simplify to if (msg->subtype < LOW_VALUE || HIGH_VALUE >
msg->subtype) in the top to bottom approach.
Regards
Matthieu
