Patches

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
Wed Apr 6 08:05:46 UTC 2016


Hi Matthew,

thanks for your patches and your interest in contributing.

On Tue, Apr 05, 2016 at 11:18:51PM +0200, Matthew Daiter wrote:
> -		if (!msc->msc_con->is_authenticated)
> -			continue;
> -		if (!is_emerg && msc->type != MSC_CON_TYPE_NORMAL)
> -			continue;
> -		if (is_emerg && !msc->allow_emerg)
> +		if ((!msc->msc_con->is_authenticated) ||
> +		    (!is_emerg && msc->type != MSC_CON_TYPE_NORMAL) ||
> +		    (is_emerg && !msc->allow_emerg))
>  			continue;

I thinkt it's a matter of taste.  To me, the existing code actually is
more obvious and easier to read, as convolutd nested parenthesis in a
single 'if' statement, where there are more possible relationship
of the individual conditions.

> -	if (memcmp(&lai, &lu->lai, sizeof(lai)) != 0) {
> +	if (memcmp(&lai, &lu->lai, sizeof(lai))) {

again here, it is a question of taste.  The '!= 0' proabably serves as
a reminder at the strange behavior of memcmp() returning 0 in case of
success.

So I'm sorry, but I wouldn't merge any of your patches, I don't think
they make the core more readable or improve it in any other way :(

I'd hope you could focus your interest in contributing into a different
area that actually makes a difference to the OpenBSC user community.
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