LAPDm code issues (ladpm.c)

[Jacob Erlbeck]

Dear Andreas,

what do you think about the other issues? My patch has not fixed all
possible problems that could arise from additional l1 headers (see below).

On 28.01.2014 15:15, Jacob Erlbeck wrote:
> 
> But AFAICS there is still something to do:
> 
> - The remaining msgb_pull() need to be checked (at least the one in
> l2_ph_data_ind() looks suspicious to me.
> - L3 length computation should be done with the macros, how it is done
> in lapdm_send_ph_data_req() is broken.
> - Why does lapd_msg_ctx have a length field that is not used?
> - ladp_test.c should be extended to check other execution paths, too.
> I've tried it for dummy packets and I didn't get it working without
> failing assertions (see below)


The following applies to most sub-projects, but I find it hard to
diagnose and use function calls if their API is not documented beyond
parameter names and types. Especially when passing a msgb to a function
it would really help if there was some documentation telling about which
parts of the msgb are used (as in "l2 is expected to point to the start
of the XYZ message"):
> - How l2/data/.. in a msg are expected to be used/set should be
> documented somewhere.

Best wishes

Jacob

> 
> ======
> 
> The following patch still breaks the assertions, at least changing the
> l3len computation in lapdm_send_ph_data_req() influences but doesn't fix
> it.
> 
> --- a/tests/lapd/lapd_test.c
> +++ b/tests/lapd/lapd_test.c
> @@ -123,8 +123,10 @@ static struct msgb *create_empty_msg(void)
>  static struct msgb *create_dummy_data_req(void)
>  {
>         struct msgb *msg;
> +       const int dummy_l1len = 3;
> 
>         msg = msgb_from_array(dummy1, sizeof(dummy1));
> +       msgb_push(msg, dummy_l1len);
>         rsl_rll_push_l3(msg, RSL_MT_DATA_REQ, 0, 0, 1);
>         return msg;
>  }
> 
> 
> 
> 
> 
> 
>