Re: [PATCH 3/6] netfilter: get rid of the grossness in netfilter.h

[Changli Gao <xiaosuo@gmail.com>]

On Wed, Nov 4, 2009 at 2:04 AM, Jan Engelhardt <jengelh@medozas.de> wrote:
>
> -#define NF_HOOK_COND(pf, hook, skb, indev, outdev, okfn, cond)                \
> -({int __ret;                                                                  \
> -if ((cond) || (__ret = nf_hook_thresh(pf, hook, (skb), indev, outdev, okfn, INT_MIN)) == 1)\
> -       __ret = (okfn)(skb);                                                   \
> -__ret;})
This code isn't the same as the linus tree's. And has a risk about
uninitialized variable __ret.

#define NF_HOOK_COND(pf, hook, skb, indev, outdev, okfn, cond)                 \
({int __ret;                                                                   \
if ((__ret=nf_hook_thresh(pf, hook, (skb), indev, outdev, okfn,
INT_MIN, cond)) == 1)\
        __ret = (okfn)(skb);                                                   \
__ret;})

/**
 *      nf_hook_thresh - call a netfilter hook
 *
 *      Returns 1 if the hook has allowed the packet to pass.  The function
 *      okfn must be invoked by the caller in this case.  Any other return
 *      value indicates the packet has been consumed by the hook.
 */
static inline int nf_hook_thresh(u_int8_t pf, unsigned int hook,
                                 struct sk_buff *skb,
                                 struct net_device *indev,
                                 struct net_device *outdev,
                                 int (*okfn)(struct sk_buff *), int thresh,
                                 int cond)
{
        if (!cond)
                return 1;
#ifndef CONFIG_NETFILTER_DEBUG
        if (list_empty(&nf_hooks[pf][hook]))
                return 1;
#endif
        return nf_hook_slow(pf, hook, skb, indev, outdev, okfn, thresh);
}


> +static inline int
> +NF_HOOK_COND(uint8_t pf, unsigned int hook, struct sk_buff *skb,
> +            struct net_device *in, struct net_device *out,
> +            int (*okfn)(struct sk_buff *), bool cond)
> +{
> +       int ret = 1;
> +       if (cond ||
> +           (ret = nf_hook_thresh(pf, hook, skb, in, out, okfn, INT_MIN) == 1))
> +               ret = okfn(skb);
> +       return ret;
> +}

The fact is: no matter the cond value, okfn(skb) should always be
called, and hf_hook_thresh() should be called only when cond is true.
So the code will be.

if (cond) {
   if (ret = nf_hook_thresh(pf, hook, skb, in, out, okfn, INT_MIN)) == 1)
       ret = okfn(skb);
} else {
    ret = okfn(skb);
}

-- 
Regards，
Changli Gao(xiaosuo@gmail.com)
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html