patch 36: application interface

This is merely a historical archive of years 2008-2021, before the migration to mailman3.

A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/OpenBSC@lists.osmocom.org/.

Harald Welte laforge at gnumonks.org
Fri Jun 12 13:11:28 UTC 2009


Dear Andreas,

I have finally found some time to review the MNCC patches in detail.

There were quite a bit of details that I wanted to change, mostly about
the code and data structure.  The logic and the idea of the interface is
still the same.

You can have a look at my code in the 'mncc-harald' branch of the new
git tree.  It's probably easiest if you look at the new mncc.h file.  Instead
of a flat list of fields, I decided for a bit more structure.  This results
in a situation where most of the ancode_foo() and decode_foo() functions
now only take a pointer to a sub-structure, rather than a long list of
arguments.  I think this is much better to understand - and also reduces
the number of lines that are either long or have to be broken in many more lines,
resulting in less code visibility on one screen page.

Furthermore, I've introduced some additional functions like mncc_set_cause()
for constantly repeated sequences of code.

Changing mncc.h obviously breaks interoperability with your lcr, and I'm
honestly sorry about that.  I'm not planning any intentional data structure
changes any time soon.

I've tested the standalone bsc_hack of the mncc-harald branch for quite some time
on a nanobts1800, it works fine.

I have some more thoughts on improving MNCC, but those changes would only
affect the implementation, not the interface.  I don't like the many explicit
lists of "if (TLVP_PRESENT(&tp, ...)) decode_something" sequences.  If we had something like a table of

========
struct parser_def {
	u_int8_t	tlv_tag;
	u_int32_t	field_flag;
	unsigned int	member_offset;
	(int)		(decode_function)(void *, u_int8_t *lv);
};

struct parser_def parser_defs[] = {
	{ GSM48_IE_SS_VERS, MNCC_F_SSVERSION,
	  offset_of(ssversion_info, struct gsm_mncc),
	  &decode_ssversion },
	...
};
========

then we could do something like have a for-loop and then only one statement
like this:
========
	struct parser_def *pd;

	if (TLVP_PRESENT(&tp, pd->tlv_tag)) {
		connect.fields |= pd->field_tag;
		pd->decode_function(&connect + pd->member_offset,
				    TLVP_VAL(&tp, pd->tlv_tag)-1);
	}
========

It would replace a lot of explicit open-coded code (38 incarnations!) with a
more data-driven model.

Regards,
-- 
- Harald Welte <laforge at gnumonks.org>           http://laforge.gnumonks.org/
============================================================================
"Privacy in residential applications is a desirable marketing option."
                                                  (ETSI EN 300 175-7 Ch. A6)




More information about the OpenBSC mailing list