Re: [dpdk-dev] [RFC 10/35] eal: introduce RTE_DECONST macro

[Olivier MATZ <olivier.matz@6wind.com>]

Hi Bruce,

On 03/10/2016 10:26 AM, Bruce Richardson wrote:
> On Thu, Mar 10, 2016 at 09:29:03AM +0100, Olivier MATZ wrote:
>>>> If you have a better alternative, without duplicating the code,
>>>> I'll be happy to learn.
>>>
>>> I really don't like this dropping of const either, but I do see the problem.
>>> I'd nearly rather see two copies of the function than start dropping the const
>>> in such a way.
>>
>> I don't think duplicating the code is a good option.
> 
> Personally, I'd actually prefer it to eliminating const-ness. I'm a big fan of
> having the compiler work for it's pay by doing typechecking for us. :-) 
> However, I would hope that by using a macro, as I suggest below, we could have
> two functions without duplicating all the code.

Does it mean we should duplicate all iterate-like functions of the
DPDK to have a const and a non-const version?
I would personally find this quite odd.


>>> Also, I'd see having the function itself be a wrapper around a
>>> macro as a better alternative too, assuming such a construction is possible.
>>
>> Sorry, I'm not sure to understand. Could you please elaborate?
>>
> The part of the code which iterates through the elements and calls a function
> for each could be a macro, which would mean that it would be fine to use the
> macro with a const mempool so long as the function being called took const
> parameters too, i.e. the type checking is done post-expansion. Basically,
> doing a multi-type function via macro (like MIN/MAX macros etc).
> 
> Haven't tried writing the code for it though, so no idea if it's actually doable
> or what the result looks like. However, at worst I would think you could 
> extract the body of the function to make it a macro, and then call it from two
> wrapper functions, one of which takes non-const param, the other of which
> takes const param. The macro itself could use typeof() internally to maintain
> const-ness or not.

OK, it's clearer, thanks.
But I'm not sure having several lines of code inside a macro
is something we should encourage either.


To summarize, I see 4 solutions:

1 do a discreet cast: I think that's what people usually do in these
  cases, I would not be suprised to find several in current DPDK
2 use a RTE_DECONST() macro: it points out that we are doing a bad cast,
  which is a valuable info for the reviewer (proof: you saw it :) )
3 duplicate the iterate functions to have a const and a non-const
  version and use a macro to duplicate the code
4 remove the const for these iterate functions, implying to remove
  const in some other functions like dumps.


I still personally prefer solution 2, because it keeps the API clean,
giving the proper information to the user and the compiler "this mempool
structure won't be modified". Using a cast like this should of course
be avoided most of the time, but I think it's acceptable in cases like
this, knowing that it is properly pointed-out by the RTE_DECONST macro.



Regards,
Olivier