Hi Feng,
2017-04-13 10:42 GMT+08:00 Gao Feng <gfree.wind@xxxxxxxxxxx>:
[...]
>> +static void
>> +__ctnetlink_change_status(struct nf_conn *ct, unsigned long on,
>> + unsigned long off)
>> +{
>> + unsigned long mask;
>> + unsigned int bit;
>> +
>> + for (bit = 0; bit < __IPS_MAX_BIT; bit++) {
>> + mask = 1 << bit;
>> + /* Ignore these unchangable bits */
>> + if (mask & IPS_UNCHANGEABLE_MASK)
>> + continue;
>
> How about clear the bits of on and off with IPS_UNCHANGEABLE_MASK before
> loop.
> Like "on &= ~ IPS_UNCHANGEABLE_MASK";
> Then the "if (mask & IPS_UNCHANGEABLE_MASK)" could be removed.
No, it's better to do this together, there are two invocations, it's not good to
copy these codes twice.
>
> BTW, when some bits are set both of on and off, the "on" would be effective,
> but "off" not.
This won't happen, see the invocation:
1. __ctnetlink_change_status(ct, status, 0);
2. __ctnetlink_change_status(ct, status, ~status);
> So I think we could use BUILD_BUG_ON to avoid it during building.
> BUILD_BUG_ON(on&mask);
Btw, this won't help, BUILD_BUG_ON is only effective on compile time,
but "on" and "off" will be modified at the running time.
--
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
