comments on jolly/lapd branch

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 -- - Harald Welte &lt;laforge(a)gnumonks.org&gt; http://laforge.gnumonks.org/ ============================================================================ "Privacy in residential applications is a desirable marketing option." (ETSI EN 300 175-7 Ch. A6)