lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Thu, 28 Jun 2012 15:35:22 +0200 From: Rostislav Lisovy <lisovy@...il.com> To: Oliver Hartkopp <socketcan@...tkopp.net> Cc: Eric Dumazet <eric.dumazet@...il.com>, netdev@...r.kernel.org, linux-can@...r.kernel.org, lartc@...r.kernel.org, pisa@....felk.cvut.cz, sojkam1@....cvut.cz Subject: Re: [PATCH net-next] em_canid: Ematch rule to match CAN frames according to their CAN IDs Hello Oliver; On Tue, 2012-06-26 at 22:07 +0200, Oliver Hartkopp wrote: > I found some time for a review. See details inline ... > I agree with quite everything except for following... > > + match = true; > > > match = 1; > egrep -r "= true;" ./linux-source | wc -l returns 6770 -- why don't you like "= true"? > > + break; > > + } > > + } > > + } else { /* SFF */ > > + can_id &= CAN_SFF_MASK; > > + match = test_bit(can_id, cm->match_sff); > > + } > > + > > > return match; > match() function must return 1 or 0, however (from my experience) test_bit() returns 0 and non-0 (strictly speaking, in my case, 0 and -1). > > + &conf[i], > > + sizeof(struct can_filter)); > > + > > + cm->eff_rules_count++; > > + } else { > > + continue; > > + } > > > omit { } around continue > http://lxr.linux.no/#linux+v3.4.4/Documentation/CodingStyle#L169 > > + } > > + > > + /* Process SFF frame rules */ > > + for (i = 0; i < cm->rules_count; i++) { > > + if ((conf[i].can_id & CAN_EFF_FLAG) && > > + (conf[i].can_mask & CAN_EFF_FLAG)) { > > > What if CAN_EFF_FLAG is set in can_id but not in can_mask ? > There were small misunderstanding from my side -- this will be rewritten in the way that EFF_FLAG in mask will determine if we care about the value of EFF_FLAG in an identifier -- i.e. when EFF_FLAG is set in the mask, the rule will be added as SFF or EFF only depending on EFF_FLAG value in the identifier. If EFF_FLAG is 0 in the mask, the rule will be added as both SFF and EFF. Regards; Rostislav -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@...r.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists