> From: Scott Mitchell <[email protected]> > > 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
