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