diff mbox series

[v3] eal: Pointer alignment check improvements

Message ID 20220922132730.5178-1-mb@smartsharesystems.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers
Series [v3] eal: Pointer alignment check improvements |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-aarch64-unit-testing success Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-x86_64-unit-testing success Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-aarch64-compile-testing success Testing PASS
ci/github-robot: build fail github build: failed
ci/iol-x86_64-compile-testing success Testing PASS
ci/intel-Testing success Testing PASS

Commit Message

Morten Brørup Sept. 22, 2022, 1:27 p.m. UTC 
  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.

v3:
- Make the uintptr_t const to avoid potential future warnings. (Bruce)
v2:
- Remove compiler attribute ((const)) from function;
  it was a coding style issue.

Signed-off-by: Morten Brørup <mb@smartsharesystems.com>
---
 lib/eal/include/rte_common.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Bruce Richardson Sept. 22, 2022, 1:38 p.m. UTC | #1 
On Thu, Sep 22, 2022 at 03:27:30PM +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.
> 
> v3:
> - Make the uintptr_t const to avoid potential future warnings. (Bruce)
> v2:
> - Remove compiler attribute ((const)) from function;
>   it was a coding style issue.
> 
> Signed-off-by: Morten Brørup <mb@smartsharesystems.com>

Acked-by: Bruce Richardson <bruce.richardson@intel.com>

Or perhaps it should be "Const-acked-by: ... " :-)
Morten Brørup Sept. 22, 2022, 8:54 p.m. UTC | #2 
> From: Bruce Richardson [mailto:bruce.richardson@intel.com]
> Sent: Thursday, 22 September 2022 15.39
> 
> On Thu, Sep 22, 2022 at 03:27:30PM +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.
> >
> > v3:
> > - Make the uintptr_t const to avoid potential future warnings.
> (Bruce)
> > v2:
> > - Remove compiler attribute ((const)) from function;
> >   it was a coding style issue.
> >
> > Signed-off-by: Morten Brørup <mb@smartsharesystems.com>
> 
> Acked-by: Bruce Richardson <bruce.richardson@intel.com>
> 
> Or perhaps it should be "Const-acked-by: ... " :-)

Unfortunately not as const as expected by both of us...

The v3 build fails at github [1] with:

../lib/eal/include/rte_common.h: In function 'int rte_is_aligned(const void*, unsigned int)':
../lib/eal/include/rte_common.h:409:27: error: type qualifiers ignored on cast result type [-Werror=ignored-qualifiers]
  409 |  return ((const uintptr_t)ptr & (align - 1)) == 0;
      |                           ^~~

[1] http://mails.dpdk.org/archives/test-report/2022-September/308604.html

I don't understand what the problem is, so my solution is omitting the const, i.e. rolling back to v2, which doesn't fail building. Unless you can suggest a better solution, Bruce?

I have changed v2 status in Patchwork back to New and v3 to Superseded.

-Morten
Bruce Richardson Sept. 23, 2022, 8:24 a.m. UTC | #3 
On Thu, Sep 22, 2022 at 10:54:38PM +0200, Morten Brørup wrote:
> > From: Bruce Richardson [mailto:bruce.richardson@intel.com]
> > Sent: Thursday, 22 September 2022 15.39
> > 
> > On Thu, Sep 22, 2022 at 03:27:30PM +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.
> > >
> > > v3:
> > > - Make the uintptr_t const to avoid potential future warnings.
> > (Bruce)
> > > v2:
> > > - Remove compiler attribute ((const)) from function;
> > >   it was a coding style issue.
> > >
> > > Signed-off-by: Morten Brørup <mb@smartsharesystems.com>
> > 
> > Acked-by: Bruce Richardson <bruce.richardson@intel.com>
> > 
> > Or perhaps it should be "Const-acked-by: ... " :-)
> 
> Unfortunately not as const as expected by both of us...
> 
> The v3 build fails at github [1] with:
> 
> ../lib/eal/include/rte_common.h: In function 'int rte_is_aligned(const void*, unsigned int)':
> ../lib/eal/include/rte_common.h:409:27: error: type qualifiers ignored on cast result type [-Werror=ignored-qualifiers]
>   409 |  return ((const uintptr_t)ptr & (align - 1)) == 0;
>       |                           ^~~
> 
> [1] http://mails.dpdk.org/archives/test-report/2022-September/308604.html
> 
> I don't understand what the problem is, so my solution is omitting the const, i.e. rolling back to v2, which doesn't fail building. Unless you can suggest a better solution, Bruce?
> 
> I have changed v2 status in Patchwork back to New and v3 to Superseded.
>
That is fine. I'll ack v2 so.
diff mbox series

Patch

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 ((const uintptr_t)ptr & (align - 1)) == 0;
 }
 
 /*********** Macros for compile type checks ********/