Hello, On Tue, Nov 27, 2012 at 02:35:05PM -0500, Aristeu Rozanski wrote: > @@ -375,22 +376,33 @@ > continue; > if (refex->access & (~ex->access)) > continue; > - match = true; > + match = 1; > break; > } > > /* > - * In two cases we'll consider this new exception valid: > - * - the dev cgroup has its default policy to allow + exception list: > - * the new exception should *not* match any of the exceptions > - * (behavior == DEVCG_DEFAULT_ALLOW, !match) > - * - the dev cgroup has its default policy to deny + exception list: > - * the new exception *should* match the exceptions > - * (behavior == DEVCG_DEFAULT_DENY, match) > + * The only three possibilities are: > + * devcg->behavior == ALLOW, rule behavior == ALLOW > + * devcg->behavior == ALLOW, rule behavior == DENY > + * devcg->behavior == DENY, rule behavior == DENY > + * the remaining > + * devcg->behavior == DENY, rule behavior == ALLOW > + * won't be possible by hierarchy > + * > + * Since we want to simplify the code, here're the possibilites to > + * make easier to understand: > + * > + * devcg behavior rule behavior match result > + * allow allow 1 0 > + * allow allow 0 1 > + * allow deny 1 0 > + * allow deny 0 1 > + * deny deny 1 1 > + * deny deny 0 0 > */ > - if ((dev_cgroup->behavior == DEVCG_DEFAULT_DENY) == match) > - return 1; > - return 0; > + if (dev_cgroup->behavior == DEVCG_DEFAULT_ALLOW) > + return !match; > + return match;
I kinda dislike this. This isn't a performanc critical path where we must try our best to shave off a few condition checks. There's no reason to encode the test like this. Please just spell the conditions out in code rather than trying to build a magic series of equality tests which somehow ends up spewing out the correct results. Thanks. -- tejun -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
