[PATCH 1/7] Add MM Auth test; add auth_action_str() function

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
Sun Mar 27 08:53:53 UTC 2016


Hi Neels,

On Sat, Mar 26, 2016 at 09:35:33PM +0100, Neels Hofmeyr wrote:
> +static inline const char *auth_action_str(enum auth_action a)

we normally try to avoid introducing some custom value/string converter
code like this and use 'struct value_string' for this.

> +#define AUTH_CASE(X) \
> +	case X: return #X

you can also define a macro like this that generates a
	{ X, #X },
for struct value_string
> +	case -1:
> +		return "(internal error)";

this would be an ugly

	{ -1, "(internal error") }

which is soon replaced by your #define AUTH_ERROR anyway.

... and please refrain from having non-trivial functions (or data
definitions), and particularly non-performance-critical functions like
this inline in a header file.

I think I know why you do that (to use it in the test case and in the
main code) but this still doesn't rectify it, sorry ;)

I would merge the entire patch-set, if you could address that one issue. Thanks!

-- 
- 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