RE: [PATCH 1/2] eal: RTE_PTR_ADD/SUB char* for compiler optimizations

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

> From: Bruce Richardson [mailto:bruce.richardson@intel.com]
> Sent: Monday, 12 January 2026 10.14
> 
> On Sun, Jan 11, 2026 at 07:59:19AM -0800, Stephen Hemminger wrote:
> > On Sun, 11 Jan 2026 10:00:32 -0500
> > scott.k.mitch1@gmail.com wrote:
> >
> > > +#define RTE_PTR_ADD(ptr, x) \
> > > +	(__extension__ ({ \
> > > +		/* Diagnostics suppressed for internal macro operations
> only. \
> > > +		 * Compiler type-checks all _Generic branches even when
> unselected, \
> > > +		 * triggering warnings with no external impact. */ \
> > > +		__rte_diagnostic_push \
> > > +		__rte_diagnostic_ignored_wcast_qual \
> > > +		_Pragma("GCC diagnostic ignored \"-Wconditional-type-
> mismatch\"") \
> > > +		/* Uses uintptr_t arithmetic for integer types (API
> compatibility), \
> > > +		 * and char* arithmetic for pointer types (enables
> optimization). */ \
> > > +		__auto_type _ptr_result = _Generic((ptr), \
> > > +			unsigned long long: ((void *)((uintptr_t)(ptr) +
> (x))), \
> > > +			long long:          ((void *)((uintptr_t)(ptr) +
> (x))), \
> > > +			unsigned long:      ((void *)((uintptr_t)(ptr) +
> (x))), \
> > > +			long:               ((void *)((uintptr_t)(ptr) +
> (x))), \
> > > +			unsigned int:       ((void *)((uintptr_t)(ptr) +
> (x))), \
> > > +			int:                ((void *)((uintptr_t)(ptr) +
> (x))), \
> > > +			unsigned short:     ((void *)((uintptr_t)(ptr) +
> (x))), \
> > > +			short:              ((void *)((uintptr_t)(ptr) +
> (x))), \
> > > +			unsigned char:      ((void *)((uintptr_t)(ptr) +
> (x))), \
> > > +			signed char:        ((void *)((uintptr_t)(ptr) +
> (x))), \
> > > +			char:               ((void *)((uintptr_t)(ptr) +
> (x))), \
> > > +			_Bool:              ((void *)((uintptr_t)(ptr) +
> (x))), \
> > > +			/* Ternary with null pointer constant: per C11, if
> one operand \
> > > +			 * is a null pointer constant and the other is a
> pointer, the \
> > > +			 * result type is qualified per the pointer operand,
> normalizing \
> > > +			 * const T* to const void* and T* to void*. */ \
> > > +			default: _Generic((1 ? (ptr) : (void *)0), \
> > > +				const void *: ((void *)((const char *)(ptr) +
> (x))), \
> > > +				default:      ((void *)((char *)(ptr) + (x))) \
> > > +			) \
> > > +		); \
> > > +		__rte_diagnostic_pop \
> > > +		_ptr_result; \
> > > +	}))
> >
> > Good idea in general but the macro is way to big and therefore hard
> to read.
> > The comments could be outside the macro.
> >
> > Any code that adds dependency on a pragma to work is brittle and
> likely
> > to allow bugs through. Please figure out how to do it without.
> 
> Do we need to handle the case of users calling RTE_PTR_ADD with integer
> values? Using this macro to essentially cast an integer to pointer
> seems
> strange. Even if it's occasionally used, I think keeping things simple
> and
> just globally changing to use "char *" is a better approach.
> 
> The only case where I'd consider trying to keep compatibility using
> uintptr_t is if the pointer parameter is a volatile one. Even then, we
> can
> probably handle that as with the "const" modifier, right?

None of the RTE_PTR_ macros can handle qualifiers (const, volatile).
Maybe it would be better to provide a new set of macros with a qualifier parameter, instead of adding new macros with e.g. _CONST in the names.