On Thu, Sep 22, 2022 at 12:52:42PM +0100, Bruce Richardson wrote:
> On Thu, Sep 22, 2022 at 01:44:13PM +0200, Morten Brørup wrote:
> > Checking a const pointer for alignment would emit a warning about the
> > const qualifier being discarded.
> >
> > No need to calculate the aligned pointer; just check the last bits of the
> > pointer.
> >
> > v2:
> > - Remove compiler attribute ((const)) from function;
> > it was a coding style issue.
> >
> > Signed-off-by: Morten Brørup <m...@smartsharesystems.com>
> > ---
> > lib/eal/include/rte_common.h | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/lib/eal/include/rte_common.h b/lib/eal/include/rte_common.h
> > index 2e22c1b955..ed81e0db0a 100644
> > --- a/lib/eal/include/rte_common.h
> > +++ b/lib/eal/include/rte_common.h
> > @@ -404,9 +404,9 @@ static void __attribute__((destructor(RTE_PRIO(prio)),
> > used)) func(void)
> > * True(1) where the pointer is correctly aligned, false(0) otherwise
> > */
> > static inline int
> > -rte_is_aligned(void *ptr, unsigned align)
> > +rte_is_aligned(const void * const __rte_restrict ptr, const unsigned int
> > align)
> > {
> > - return RTE_PTR_ALIGN(ptr, align) == ptr;
> > + return ((uintptr_t)ptr & (align - 1)) == 0;
>
> Are we confident that in future, or using come compiler settings, we won't
> get an error due to using "uintptr_t" rather than "const uintptr_t" in the
> cast? I would put a const in there myself, just to be safe.
>
> A further point, only-semi-related to this patch, which is fine as-is:
> looking at the code for the various macros in rte_common.h:
> * The various macros for working on pointers can can probably be converted
> to functions, since they don't need to work with variable-sized types.
> * We can then see about properly ensuring those inline functions are
> const-correct.
>
Actually, on further investigation in trying this, it appears that the
macros are used in a number of places with integer data too, despite the
"PTR" in the name, so things are best alone for now, I think.
/Bruce
