Re: [PATCH 6/6] netfilter: xtables: add const qualifiers

[Patrick McHardy <kaber@trash.net>]

Alexey Dobriyan wrote:
> On Thu, Feb 11, 2010 at 11:27 AM, Patrick McHardy <kaber@trash.net> wrote:
>> Alexey Dobriyan wrote:
>>>> -static inline int ebt_basic_match(struct ebt_entry *e, struct ethhdr *h,
>>>> -   const struct net_device *in, const struct net_device *out)
>>>> +static inline int
>>>> +ebt_basic_match(const struct ebt_entry *e, const struct ethhdr *h,
>>>> +                const struct net_device *in, const struct net_device *out)
>>> These const modifiers are pointless because compilers are smart enough
>>> to notice non-modifiability and generate the very same code in both cases.
>>>
>>> Nowadays, half of functions declarations in generic
>>> xtables/iptables/ip6tables/arptables
>>> code are littered with const which makes them pretty unpleasant to read.
>> Well, the benefit is that the compiler can catch accidental modification
>> of data that is supposed to be constant.
> 
> Did it ever? Realistically?

Yes, occasionally it has caught typos or other variable mixups for me.
It also documents that a variable is only supposed to be read.

>>From what I see, const is used with string functions
> (to make string literals go to rodata) and with structures full of hooks
> (to make them go to rodata).

Yes, that's also a reason.

>>>> -    struct ebt_entry *e = (struct ebt_entry *)chain->data;
>>>> +    const struct ebt_entry *e = (struct ebt_entry *)chain->data;
>>> And such things are wrong (not second const).
>> What's wrong about this?
> 
> Should it be (const struct ebt_entry *)?

That's not necessary. Its just declaring the the memory should
not be modified through e. The const qualification happens
automatically.