Hi Andreas,

I've done a quick read through your jolly/lapd branches, and I have the following comments:

1) great work, thanks :)

2) lapd_profile I would like to use a 'struct lapd_profile' instead of a simple enum. Why? Because this puts all the values/parameters (like k, n201, short_address, use_sabme, t200, etc.) into the control of the caller, instead of the library. so basically we would have struct lapd_profile { uint8_t k; unsigned int n201; ... }; and then some library-pre-defined ones: const struct lapd_profile lapd_profile_isdn = { ... }; const struct lapd_profile lapd_profile_abis = { ... }; but this way an application could define their own profile with slightly different timers, without having to modify the library codebase.

The actual lapd_datalink then just would have a const struct lapd_profile *profile; member that is assigned during lapd_dl_init().

3) the msgb_free() changes in input/dahdi.c, src/e1_input.c, etc. should be a separate patch, as they are actual bugfixes of the old code, as far as I understand. That bugfix patch should be first, then the actual LAPD restructuring on top of it.

4) regarding the two-dimensional switch() statements, I suggest introducing the following macros to include/osmocom/core/osmo_prim.h:

#define OSMO_PRIM(prim, op) (prim << 8 | op & 0xFF) #define OSMO_PRIM_HDR(oph) OSMO_PRIM(oph->primitive, oph->operation)

then you can do something like this in the lapd code:

switch (OSMO_PRIM_HDR(oph)) { case OSMO_PRIM(PRIM_DL_EST, PRIM_OP_REQUEST): ... }

Regards, Harald