On Tue, Jun 07, 2016 at 10:04:40AM -0700, Joe Perches wrote:
> On Tue, 2016-06-07 at 17:14 +0200, Pablo Neira Ayuso wrote:
> > On Tue, May 10, 2016 at 11:26:56AM +1000, tcharding wrote:
> > > From: Tobin C Harding <m...@tobin.cc>
> > > This is my second linux kernel patch. Unsure if I was meant to cc
> > > multiple mailing lists?
> []
> > > diff --git a/net/bridge/netfilter/ebt_stp.c
> > > b/net/bridge/netfilter/ebt_stp.c
> []
> > > @@ -55,65 +55,65 @@ static bool ebt_filter_config(const struct
> > > ebt_stp_info *info,
> > > if (info->bitmask & EBT_STP_ROOTPRIO) {
> > > v16 = NR16(stpc->root);
> > > if (FWINV(v16 < c->root_priol ||
> > > - v16 > c->root_priou, EBT_STP_ROOTPRIO))
> > > + v16 > c->root_priou, EBT_STP_ROOTPRIO))
> > I don't think this coding style is right. This is a common approach
> > (to align the condition when split in several lines) in other 'net' code.
>
> Perhaps you misread the code.
Oh right. This FWINV() got me confused.
> The alignment is changed for the 1st argument of the FWINV macro
> to be more similar to the style used in the rest of net/
>
> But using a longer initial line would be more readable:
>
> if (FWINV(v16 < c->root_priol || v16 > c->root_priou,
> EBT_STP_ROOTPRIO))
I see. Thanks for clarifying all the FWINV() related changes.
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.
