Jan Engelhardt wrote: > On Tuesday 2010-03-23 15:44, Patrick McHardy wrote: > >> Jan Engelhardt wrote: >> >>> diff --git a/net/ipv4/netfilter/ipt_LOG.c b/net/ipv4/netfilter/ipt_LOG.c >>> index c9ee5c4..33213d6 100644 >>> --- a/net/ipv4/netfilter/ipt_LOG.c >>> +++ b/net/ipv4/netfilter/ipt_LOG.c >>> @@ -445,7 +445,7 @@ static int log_tg_check(const struct xt_tgchk_param *par) >>> >>> if (loginfo->level >= 8) { >>> pr_debug("level %u >= 8\n", loginfo->level); >>> - return false; >>> + return -EDOM; >>> >>> >> Mhh is that really a "Math argument out of domain of func"? ERANGE seems >> slightly >> better fitting to me. >> > > I had to look up the difference too, but it's pretty obvious > (to me at least). > > EDOM is for when a function is not defined at a point, such as log(0) > or divide(1.0, 0.0). Of course you could say "that's ±Inf", but that > infinity argument I'll leave to academia. > > However, "2^63 * 4" for example is well-defined in maths, but > computers are the limiting factor, hence ERANGE is appropriate in > this case. > > > >>> { >>> + int ret; >>> + >>> if (XT_ALIGN(par->match->matchsize) != size && >>> par->match->matchsize != -1) { >>> /* >>> @@ -399,8 +401,13 @@ int xt_check_match(struct xt_mtchk_param *par, >>> par->match->proto); >>> return -EINVAL; >>> } >>> - if (par->match->checkentry != NULL && !par->match->checkentry(par)) >>> - return -EINVAL; >>> + if (par->match->checkentry != NULL) { >>> + ret = par->match->checkentry(par); >>> + if (ret < 0) >>> + return ret; >>> + else if (ret == 0) >>> + return -EINVAL; >>> >> I'd suggest to do the true/false conversion before this patch. Currently I >> don't understand the split of these patches, this one keeps lots of boolean >> return values and initializations, but changes some. >> > > I did this so that the two main changes, > > A1. replacing 'return false' by "detailed" code > (EDOM, etc.) > > A2. replacing all remaining bools that did not get a detailed_code > by EINVAL. > > It is also possible to change the order, as in > > B1. change everything to EINVAL first > > B2. then change EINVAL -> detailed_code. > > Should I switch? Yes, I think that would ease review. Thanks. -- 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