Skip to contentSkip to conversation

eal / malloc : alignment parameter check failing due to changes in rte_is_power_of_2

PATCH In review Declan Doherty 3 participants 5 messages Jan 16, 2015 – Jan 19, 2015 2 messages not in the archive

lore
Share & tools
lore

Declan Doherty

author
#0 lore raw

PATCH 1/1 eal / malloc : alignment parameter check failing due to changes in rte_is_power_of_2

In commit 2fc8d6d the behaviour of function rte_is_power_of_2 was changed to not return true for 0. memzone_reserve_aligned_thread_unsafe and rte_malloc_socket both make the assumption that for align = 0 !rte_is_power_of_2(align) will return false. This patch adds a check that align parameter is non-zero before doing the power of 2 check

Signed-off-by Declan Doherty <[email protected]>
---
 lib/librte_eal/common/eal_common_memzone.c | 2 +-
 lib/librte_malloc/rte_malloc.c             | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)
lib/librte_eal/common/eal_common_memzone.c +1 -1
index b5a5d72..3f09338 100644
@@ -156,7 +156,7 @@ memzone_reserve_aligned_thread_unsafe(const char *name, size_t len,
 	}
 
 	/* if alignment is not a power of two */
-	if (!rte_is_power_of_2(align)) {
+	if (align ? !rte_is_power_of_2(align) : 0) {
 		RTE_LOG(ERR, EAL, "%s(): Invalid alignment: %u\n", __func__,
 				align);
 		rte_errno = EINVAL;
lib/librte_malloc/rte_malloc.c +1 -1
index b966fc7..15c7e20 100644
@@ -75,7 +75,7 @@ rte_malloc_socket(const char *type, size_t size, unsigned align, int socket_arg)
 	void *ret;
 
 	/* return NULL if size is 0 or alignment is not power-of-2 */
-	if (size == 0 || !rte_is_power_of_2(align))
+	if (size == 0 || align ? !rte_is_power_of_2(align) : 0)
 		return NULL;
 
 	if (socket_arg == SOCKET_ID_ANY)
Signature · 1 line
1.7.12.2

A message that is not in the archive (sent off-list or not yet received): 1421421030-22261-1-git-send-email-declan.doherty-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org

Acked-by
#2 lore raw
Quoted 9 lines

-----Original Message-----
From: dev [mailto:[email protected]] On Behalf Of Declan Doherty
Sent: Friday, January 16, 2015 3:11 PM
To: [email protected]
Subject: [dpdk-dev] [PATCH] eal / malloc : alignment parameter check failing due to changes in rte_is_power_of_2

Quoted 7 lines

In commit 2fc8d6d the behaviour of function rte_is_power_of_2 was changed to not return true for 0. memzone_reserve_aligned_thread_unsafe and rte_malloc_socket both make the assumption that for align = 0 !rte_is_power_of_2(align) will return false. This patch adds a check that align parameter is non-zero before doing the power of 2 check

Signed-off-by Declan Doherty <[email protected]>
Acked-by Pablo de Lara <[email protected]>
#3 lore raw
In reply to a message not in the archive

2015-01-16 15:10, Declan Doherty:

Quoted 7 lines

In commit 2fc8d6d the behaviour of function rte_is_power_of_2 was changed to not return true for 0. memzone_reserve_aligned_thread_unsafe and rte_malloc_socket both make the assumption that for align = 0 !rte_is_power_of_2(align) will return false. This patch adds a check that align parameter is non-zero before doing the power of 2 check

Signed-off-by Declan Doherty <[email protected]>

[...]

Quoted 2 lines
lib/librte_eal/common/eal_common_memzone.c +1 -1
-	if (!rte_is_power_of_2(align)) {
+	if (align ? !rte_is_power_of_2(align) : 0) {

[...]

lib/librte_malloc/rte_malloc.c rte_malloc_socket(const char *type, size_t size, unsigned align, int socket_arg) · line 78

Quoted 2 lines
lib/librte_malloc/rte_malloc.c +1 -1
-	if (size == 0 || !rte_is_power_of_2(align))
+	if (size == 0 || align ? !rte_is_power_of_2(align) : 0)

I don't understand why you write "align ? !rte_is_power_of_2(align) : 0" instead of the more readable "align && !rte_is_power_of_2(align)" ?

Pablo acked it so I guess there is something obvious I'm missing.

Signature · 1 line
Thomas
author
#4 lore raw
Quoted 26 lines

-----Original Message-----
From: Thomas Monjalon [mailto:[email protected]]
Sent: Friday, January 16, 2015 5:02 PM
To: Doherty, Declan
Cc: [email protected]; De Lara Guarch, Pablo
Subject: Re: [dpdk-dev] [PATCH] eal / malloc : alignment parameter check failing due to changes in rte_is_power_of_2

2015-01-16 15:10, Declan Doherty:

Quoted 7 lines

In commit 2fc8d6d the behaviour of function rte_is_power_of_2 was changed to not return true for 0. memzone_reserve_aligned_thread_unsafe and rte_malloc_socket both make the assumption that for align = 0 !rte_is_power_of_2(align) will return false. This patch adds a check that align parameter is non-zero before doing the power of 2 check

Signed-off-by Declan Doherty <[email protected]>

[...]

Quoted 2 lines
lib/librte_eal/common/eal_common_memzone.c +1 -1
-	if (!rte_is_power_of_2(align)) {
+	if (align ? !rte_is_power_of_2(align) : 0) {

[...]

lib/librte_malloc/rte_malloc.c rte_malloc_socket(const char *type, size_t size, unsigned align, int socket_arg) · line 78

Quoted 2 lines
lib/librte_malloc/rte_malloc.c +1 -1
-	if (size == 0 || !rte_is_power_of_2(align))
+	if (size == 0 || align ? !rte_is_power_of_2(align) : 0)

I don't understand why you write "align ? !rte_is_power_of_2(align) : 0" instead of the more readable "align && !rte_is_power_of_2(align)" ?

Quoted 4 lines

Pablo acked it so I guess there is something obvious I'm missing.

--
Thomas

No there's nothing you're missing, this is just the way I saw the logic, if align is none zero, then test the power of 2 condition otherwise return 0. I have no problem with your suggestion in you prefer that, at the end of the day the logic test works out equivalent.

A message that is not in the archive (sent off-list or not yet received): 345C63BAECC1AD42A2EC8C63AFFC3ADC2747EE3D-kPTMFJFq+rF9qrmMLTLiibfspsVTdybXVpNB7YpNyf8@public.gmane.org

#6 lore raw

2015-01-18 19:26, Doherty, Declan:

Quoted 8 lines

From: Thomas Monjalon [mailto:[email protected]]

Quoted 7 lines

2015-01-16 15:10, Declan Doherty:

Quoted 6 lines

In commit 2fc8d6d the behaviour of function rte_is_power_of_2 was changed to not return true for 0. memzone_reserve_aligned_thread_unsafe and rte_malloc_socket both make the assumption that for align = 0 !rte_is_power_of_2(align) will return false. This patch adds a check that align parameter is non-zero before doing the power of 2 check

Quoted 7 lines
Quoted 7 lines
Quoted 1 line
Signed-off-by Declan Doherty <[email protected]>

[...]

Quoted 2 lines
lib/librte_eal/common/eal_common_memzone.c +1 -1
-	if (!rte_is_power_of_2(align)) {
+	if (align ? !rte_is_power_of_2(align) : 0) {

[...]

lib/librte_malloc/rte_malloc.c rte_malloc_socket(const char *type, size_t size, unsigned align, int socket_arg) · line 78

Quoted 2 lines
lib/librte_malloc/rte_malloc.c +1 -1
-	if (size == 0 || !rte_is_power_of_2(align))
+	if (size == 0 || align ? !rte_is_power_of_2(align) : 0)

There is an operator precedence bug here.
Parens are needed after ||.

Quoted 8 lines
Quoted 4 lines

I don't understand why you write "align ? !rte_is_power_of_2(align) : 0" instead of the more readable "align && !rte_is_power_of_2(align)" ?

Pablo acked it so I guess there is something obvious I'm missing.

No there's nothing you're missing, this is just the way I saw the logic, if align is none zero, then test the power of 2 condition otherwise return 0. I have no problem with your suggestion in you prefer that, at the end of the day the logic test works out equivalent.

So I change to the simpler && form and I add parentheses after || to correctly check size==0.

Applied with above changes.

Thanks

Signature · 1 line
Thomas