Fix a warning when the rte_common.h header is included in a compilation using -Wbad-function-cast, such as in Open vSwitch where the following warning is emitted repeatedly: ../rte_common.h: In function 'rte_is_aligned': ../rte_common.h:184:9: warning: cast from function call of type 'uintptr_t' to non-matching type 'void *' [-Wbad-function-cast] This change fixes the issue in rte_common.h by using the RTE_ALIGN_FLOOR macro to get the aligned floor value with generic type casting. Signed-off-by: John McNamara <john.mcnamara@intel.com> --- lib/librte_eal/common/include/rte_common.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/librte_eal/common/include/rte_common.h b/lib/librte_eal/common/include/rte_common.h index 4971049..86b7e9b 100644 --- a/lib/librte_eal/common/include/rte_common.h +++ b/lib/librte_eal/common/include/rte_common.h @@ -120,7 +120,7 @@ rte_align_floor_int(uintptr_t ptr, uintptr_t align) * must be a power-of-two value. */ #define RTE_PTR_ALIGN_FLOOR(ptr, align) \ - (typeof(ptr))rte_align_floor_int((uintptr_t)ptr, align) + (typeof(ptr))RTE_ALIGN_FLOOR((uintptr_t)ptr, align) /** * Macro to align a value to a given power-of-two. The resultant value -- 1.8.1.4
On Mon, Mar 16, 2015 at 12:56:04PM +0000, John McNamara wrote:
> Fix a warning when the rte_common.h header is included in a compilation
> using -Wbad-function-cast, such as in Open vSwitch where the
> following warning is emitted repeatedly:
>
> ../rte_common.h: In function 'rte_is_aligned':
> ../rte_common.h:184:9: warning: cast from function call of
> type 'uintptr_t' to non-matching type 'void *' [-Wbad-function-cast]
>
> This change fixes the issue in rte_common.h by using the RTE_ALIGN_FLOOR
> macro to get the aligned floor value with generic type casting.
>
> Signed-off-by: John McNamara <john.mcnamara@intel.com>
> ---
> lib/librte_eal/common/include/rte_common.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/lib/librte_eal/common/include/rte_common.h b/lib/librte_eal/common/include/rte_common.h
> index 4971049..86b7e9b 100644
> --- a/lib/librte_eal/common/include/rte_common.h
> +++ b/lib/librte_eal/common/include/rte_common.h
> @@ -120,7 +120,7 @@ rte_align_floor_int(uintptr_t ptr, uintptr_t align)
> * must be a power-of-two value.
> */
> #define RTE_PTR_ALIGN_FLOOR(ptr, align) \
> - (typeof(ptr))rte_align_floor_int((uintptr_t)ptr, align)
> + (typeof(ptr))RTE_ALIGN_FLOOR((uintptr_t)ptr, align)
>
> /**
> * Macro to align a value to a given power-of-two. The resultant value
> --
> 1.8.1.4
>
>
This looks reasonable, but it rather begs the question as to why we need
rte_align_floor_int in the first place. Theres only one other call site, and it
looks like it could use RTE_PTR_ALIGN_FLOOR just as easily. What about fixing
up the second call site and removing the function to save some space?
Best
Neil
> -----Original Message-----
> From: Neil Horman [mailto:nhorman@tuxdriver.com]
> Sent: Monday, March 16, 2015 1:19 PM
> To: Mcnamara, John
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH] eal: fix Wbad-function-cast warning
>
>
> This looks reasonable, but it rather begs the question as to why we need
> rte_align_floor_int in the first place. Theres only one other call site,
> and it looks like it could use RTE_PTR_ALIGN_FLOOR just as easily. What
> about fixing up the second call site and removing the function to save
> some space?
Hi Neil,
Seems like a good idea. I'll submit a v2.
Does rte_align_floor_int() need to be deprecated in some way or is it okay to just remove it?
John
On Mon, Mar 16, 2015 at 03:04:45PM +0000, Mcnamara, John wrote:
> > -----Original Message-----
> > From: Neil Horman [mailto:nhorman@tuxdriver.com]
> > Sent: Monday, March 16, 2015 1:19 PM
> > To: Mcnamara, John
> > Cc: dev@dpdk.org
> > Subject: Re: [dpdk-dev] [PATCH] eal: fix Wbad-function-cast warning
> >
> >
> > This looks reasonable, but it rather begs the question as to why we need
> > rte_align_floor_int in the first place. Theres only one other call site,
> > and it looks like it could use RTE_PTR_ALIGN_FLOOR just as easily. What
> > about fixing up the second call site and removing the function to save
> > some space?
>
> Hi Neil,
>
> Seems like a good idea. I'll submit a v2.
>
> Does rte_align_floor_int() need to be deprecated in some way or is it okay to just remove it?
>
> John
>
After the 2.0 release you should put a note in the api document, yes, but right
now its not required. Though if you wanted to, I wouldn't stop you :)
Neil
Fix a warning when the rte_common.h header is included in a compilation using -Wbad-function-cast, such as in Open vSwitch where the following warning is emitted repeatedly: ../rte_common.h: In function 'rte_is_aligned': ../rte_common.h:184:9: warning: cast from function call of type 'uintptr_t' to non-matching type 'void *' [-Wbad-function-cast] This change fixes the issue in rte_common.h by using the RTE_ALIGN_FLOOR macro to get the aligned floor value with generic type casting. Also removed the rte_align_floor_int() function and replaced it with the RTE_PTR_ALIGN_FLOOR() macro. Signed-off-by: John McNamara <john.mcnamara@intel.com> --- app/test/test_common.c | 4 ++-- lib/librte_eal/common/include/rte_common.h | 21 +-------------------- lib/librte_malloc/malloc_elem.c | 2 +- 3 files changed, 4 insertions(+), 23 deletions(-) diff --git a/app/test/test_common.c b/app/test/test_common.c index 4b71e7b..66e9109 100644 --- a/app/test/test_common.c +++ b/app/test/test_common.c @@ -122,8 +122,8 @@ test_align(void) for (i = 1; i <= MAX_NUM; i++) { /* align floor */ - if (rte_align_floor_int((uintptr_t)i, p) % p) - FAIL_ALIGN("rte_align_floor_int", i, p); + if (RTE_ALIGN_FLOOR((uintptr_t)i, p) % p) + FAIL_ALIGN("RTE_ALIGN_FLOOR", i, p); val = RTE_PTR_ALIGN_FLOOR((uintptr_t) i, p); if (ERROR_FLOOR(val, i, p)) diff --git a/lib/librte_eal/common/include/rte_common.h b/lib/librte_eal/common/include/rte_common.h index 4971049..c0ab8b4 100644 --- a/lib/librte_eal/common/include/rte_common.h +++ b/lib/librte_eal/common/include/rte_common.h @@ -93,25 +93,6 @@ extern "C" { /*********** Macros/static functions for doing alignment ********/ -/** - * Function which rounds an unsigned int down to a given power-of-two value. - * Takes uintptr_t types as parameters, as this type of operation is most - * commonly done for pointer alignment. (See also RTE_ALIGN_FLOOR, - * RTE_ALIGN_CEIL, RTE_ALIGN, RTE_PTR_ALIGN_FLOOR, RTE_PTR_ALIGN_CEL, - * RTE_PTR_ALIGN macros) - * @param ptr - * The value to be rounded down - * @param align - * The power-of-two of which the result must be a multiple. - * @return - * Function returns a properly aligned value where align is a power-of-two. - * If align is not a power-of-two, result will be incorrect. - */ -static inline uintptr_t -rte_align_floor_int(uintptr_t ptr, uintptr_t align) -{ - return (ptr & ~(align - 1)); -} /** * Macro to align a pointer to a given power-of-two. The resultant @@ -120,7 +101,7 @@ rte_align_floor_int(uintptr_t ptr, uintptr_t align) * must be a power-of-two value. */ #define RTE_PTR_ALIGN_FLOOR(ptr, align) \ - (typeof(ptr))rte_align_floor_int((uintptr_t)ptr, align) + ((typeof(ptr))RTE_ALIGN_FLOOR((uintptr_t)ptr, align)) /** * Macro to align a value to a given power-of-two. The resultant value diff --git a/lib/librte_malloc/malloc_elem.c b/lib/librte_malloc/malloc_elem.c index fd0532e..a5e1248 100644 --- a/lib/librte_malloc/malloc_elem.c +++ b/lib/librte_malloc/malloc_elem.c @@ -90,7 +90,7 @@ elem_start_pt(struct malloc_elem *elem, size_t size, unsigned align) { const uintptr_t end_pt = (uintptr_t)elem + elem->size - MALLOC_ELEM_TRAILER_LEN; - const uintptr_t new_data_start = rte_align_floor_int((end_pt - size),align); + const uintptr_t new_data_start = RTE_ALIGN_FLOOR((end_pt - size), align); const uintptr_t new_elem_start = new_data_start - MALLOC_ELEM_HEADER_LEN; /* if the new start point is before the exist start, it won't fit */ -- 1.8.1.4
On Mon, Mar 16, 2015 at 05:05:06PM +0000, John McNamara wrote:
> Fix a warning when the rte_common.h header is included in a compilation
> using -Wbad-function-cast, such as in Open vSwitch where the
> following warning is emitted repeatedly:
>
> ../rte_common.h: In function 'rte_is_aligned':
> ../rte_common.h:184:9: warning: cast from function call of
> type 'uintptr_t' to non-matching type 'void *' [-Wbad-function-cast]
>
> This change fixes the issue in rte_common.h by using the RTE_ALIGN_FLOOR
> macro to get the aligned floor value with generic type casting.
>
> Also removed the rte_align_floor_int() function and replaced it with
> the RTE_PTR_ALIGN_FLOOR() macro.
>
> Signed-off-by: John McNamara <john.mcnamara@intel.com>
Acked-by: Neil Horman <nhorman@tuxdriver.com>
2015-03-16 13:52, Neil Horman:
> On Mon, Mar 16, 2015 at 05:05:06PM +0000, John McNamara wrote:
> > Fix a warning when the rte_common.h header is included in a compilation
> > using -Wbad-function-cast, such as in Open vSwitch where the
> > following warning is emitted repeatedly:
> >
> > ../rte_common.h: In function 'rte_is_aligned':
> > ../rte_common.h:184:9: warning: cast from function call of
> > type 'uintptr_t' to non-matching type 'void *' [-Wbad-function-cast]
> >
> > This change fixes the issue in rte_common.h by using the RTE_ALIGN_FLOOR
> > macro to get the aligned floor value with generic type casting.
> >
> > Also removed the rte_align_floor_int() function and replaced it with
> > the RTE_PTR_ALIGN_FLOOR() macro.
> >
> > Signed-off-by: John McNamara <john.mcnamara@intel.com>
>
> Acked-by: Neil Horman <nhorman@tuxdriver.com>
Applied, thanks