Re: [PATCH] netfilter: xt_hashlimit: fix inconsistent return type in hashlimit_mt_*

[Florian Westphal]

Miaoqian Lin <linmq006@xxxxxxxxx> wrote:
> The hashlimit_mt_v1() and hashlimit_mt_v2() functions return the
> cfg_copy() error code (-EINVAL) instead of false when configuration
> copying fails. Since these functions are declared to return bool,
> -EINVAL is interpreted as true, which is misleading.

Could you please check if its possible to rework cfg_copy() to not
return anything?

> --- a/net/netfilter/xt_hashlimit.c
> +++ b/net/netfilter/xt_hashlimit.c
> @@ -806,7 +806,7 @@ hashlimit_mt_v1(const struct sk_buff *skb, struct xt_action_param *par)
>  
>  	ret = cfg_copy(&cfg, (void *)&info->cfg, 1);
>  	if (ret)
> -		return ret;
> +		return false;

AFAICS cfg_copy cannot return an error.

You could try adding an enum for the version field to xt_hashlimit.c,
then use switch/case to let compiler complain for other values.

Or try to replace the else branch error return with BUILD_BUG(),
compiler should be able to figure this out.

You might have to add __always_inline hint.