On Wed, 2016-06-08 at 13:52 +0200, Pablo Neira Ayuso wrote: > On Tue, Jun 07, 2016 at 11:02:30AM -0700, Joe Perches wrote: > > On Tue, 2016-06-07 at 19:34 +0200, Pablo Neira Ayuso wrote: > > > On Tue, Jun 07, 2016 at 10:04:40AM -0700, Joe Perches wrote: > > > > One more question, is this chunk below correct from > > > > coding style point of view? > > > if (info->bitmask & EBT_STP_ROOTADDR) { > > > verdict = 0; > > > for (i = 0; i < 6; i++) > > > - verdict |= (stpc->root[2+i] ^ c->root_addr[i]) & > > > - c->root_addrmsk[i]; > > > + verdict |= (stpc->root[2 + i] ^ c->root_addr[i]) & > > > + c->root_addrmsk[i]; > > > > > > I think the previous line is fine. > > "2+i" or "2 + i", either is OK. > > Multiple line statement alignment doesn't > > matter much. > Sorry, I was actually refering to: [] Hi again Pablo. No worries. I hoped the "doesn't matter much" was clear enough. There are many different multiple line statement alignment styles in the kernel. Alignment to open parenthesis is one of them, and I think it's reasonable to standardize on that. For multiple line statements without parentheses for alignment, I think there isn't one style that's much better than another. I slightly prefer the original alignment above myself. > > Perhaps it's better to add a function for this though. > I like this function idea :). Maybe something like this is clearer: static bool ebt_test_addr(const uint8_t *root, const char *addr, const char *mask) { int i; for (i = 0; i < ETH_ALEN; i++) { if ((root[2 + i] ^ addr[i]) & mask[i]) return true; } return false; } Maybe the call should add the + 2 to the first argument instead of using + 2 in the loop. -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html