RE: [PATCH v14] eal: RTE_PTR_ADD/SUB API improvements

["Morten Brørup" <mb@smartsharesystems.com>]

> From: Scott Mitchell <scott.k.mitch1@gmail.com>
> 
> RTE_PTR_ADD and RTE_PTR_SUB APIs have a few limitations:
> 1. ptr cast to uintptr_t drops pointer provenance and
> prevents compiler optimizations
> 2. return cast discards qualifiers (const, volatile)
> which may hide correctness/concurrency issues.
> 3. Accepts both "pointers" and "integers as pointers" which
> overloads the use case and constrains the implementation
> to address other challenges.
> 
> This patch splits the API on two dimensions:
> 1. pointer types
> 2. integer types that represent pointers
> 
> This split allows addressing each of the challenges above
> and provides distinct APIs for the distinct use cases.

First of all, I think it's a huge improvement of the RTE_PTR_ADD/SUB macros to not discard qualifiers anymore.
Silently discarding qualifiers will cause bugs (discarding volatile) and potentially degrade performance (discarding const).

And I agree with your approach of generally fixing instances where this qualifier-discarding property is abused across existing code.
Qualifiers should only be explicitly discarded.

Unfortunately, fixing the macros changes their behavior, which may be considered API breakage.
Personally, I consider the qualifier-discarding property of the current macros a bug, which makes your patch a bug fix.
I would like the opinion of other community members on this!
It seems a patch can be both a bug fix and an API break. ;-)

Second, I think RTE_INT_PTR_ADD/SUB macros are superfluous.
Just use standard + and - operations when operating on integers, including "integers as pointers".

I guess existing instances of RTE_PTR_ADD/SUB operating on "integers as pointers" can be fixed across existing code to either use standard +/- operators or - probably more correct - use actual pointer types instead of integer types.

<feature creep>
If we decide to proceed with the RTE_PTR_ADD/SUB macros not discarding qualifiers, similar RTE_PTR_ macros should be updated similarly.
But let's cross that bridge if we go there.
</feature creep>

-Morten