[dpdk-dev] debug: update assertion macro

Message ID 9B0F484B-2803-4882-B324-8E4B1AFEFAF2@gmail.com (mailing list archive)
State Accepted, archived
Delegated to: Ferruh Yigit
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Ilya Matveychikov Nov. 19, 2017, 8:18 a.m. UTC
  Update RTE_VERIFY macro to make it possible to use complex expressions
in RTE_ASSERT.

Signed-off-by: Ilya V. Matveychikov <matvejchikov@gmail.com>

Fixes: 148f963fb532 ("xen: core library changes")
Cc: bruce.richardson@intel.com

---
Now it's incorrect to use complex expressions for assertion
like RTE_ASSERT((1 + 2) == 3). This patch makes it possible.

 lib/librte_eal/common/include/rte_debug.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--
2.15.0
  

Comments

Ilya Matveychikov Nov. 19, 2017, 2:41 p.m. UTC | #1
> On Nov 19, 2017, at 12:18 PM, Ilya Matveychikov <matvejchikov@gmail.com> wrote:
> 
> Update RTE_VERIFY macro to make it possible to use complex expressions
> in RTE_ASSERT.
> 
> Signed-off-by: Ilya V. Matveychikov <matvejchikov@gmail.com>
> 
> Fixes: 148f963fb532 ("xen: core library changes")
> Cc: bruce.richardson@intel.com
> 
> ---
> Now it's incorrect to use complex expressions for assertion
> like RTE_ASSERT((1 + 2) == 3). This patch makes it possible.

Update.

Now it’s possible to have % char inside the expression, for example:

RTE_ASSERT((sizeof(some_struct) % 64) == 0)

Before the patch, “%" sign acts like a conversion specification beginning
character.

> 
> lib/librte_eal/common/include/rte_debug.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lib/librte_eal/common/include/rte_debug.h b/lib/librte_eal/common/include/rte_debug.h
> index 79b67b3ec..fbb3bb5e5 100644
> --- a/lib/librte_eal/common/include/rte_debug.h
> +++ b/lib/librte_eal/common/include/rte_debug.h
> @@ -86,7 +86,7 @@ void rte_dump_registers(void);
> #endif
> #define	RTE_VERIFY(exp)	do {                                                  \
> 	if (unlikely(!(exp)))                                                           \
> -		rte_panic("line %d\tassert \"" #exp "\" failed\n", __LINE__); \
> +		rte_panic("line %d\tassert \"%s\" failed\n", __LINE__, #exp); \
> } while (0)
> 
> /*
> --
> 2.15.0
  
Ferruh Yigit Dec. 15, 2017, 7:56 p.m. UTC | #2
On 11/19/2017 6:41 AM, Ilya Matveychikov wrote:
> 
> 
>> On Nov 19, 2017, at 12:18 PM, Ilya Matveychikov <matvejchikov@gmail.com> wrote:
>>
>> Update RTE_VERIFY macro to make it possible to use complex expressions
>> in RTE_ASSERT.
>>
>> Signed-off-by: Ilya V. Matveychikov <matvejchikov@gmail.com>
>>
>> Fixes: 148f963fb532 ("xen: core library changes")
>> Cc: bruce.richardson@intel.com
>>
>> ---
>> Now it's incorrect to use complex expressions for assertion
>> like RTE_ASSERT((1 + 2) == 3). This patch makes it possible.
> 
> Update.
> 
> Now it’s possible to have % char inside the expression, for example:
> 
> RTE_ASSERT((sizeof(some_struct) % 64) == 0)
> 
> Before the patch, “%" sign acts like a conversion specification beginning
> character.

previous sample is not correct, complex expressions "RTE_ASSERT((1 + 2) == 3)"
are working fine.
But you are right "%" causing problem, and this patch is fixing it.

Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>
  
Ferruh Yigit Dec. 22, 2017, 2:12 a.m. UTC | #3
On 12/15/2017 11:56 AM, Ferruh Yigit wrote:
> On 11/19/2017 6:41 AM, Ilya Matveychikov wrote:
>>
>>
>>> On Nov 19, 2017, at 12:18 PM, Ilya Matveychikov <matvejchikov@gmail.com> wrote:
>>>
>>> Update RTE_VERIFY macro to make it possible to use complex expressions
>>> in RTE_ASSERT.
>>>
>>> Signed-off-by: Ilya V. Matveychikov <matvejchikov@gmail.com>
>>>
>>> Fixes: 148f963fb532 ("xen: core library changes")
>>> Cc: bruce.richardson@intel.com
>>>
>>> ---
>>> Now it's incorrect to use complex expressions for assertion
>>> like RTE_ASSERT((1 + 2) == 3). This patch makes it possible.
>>
>> Update.
>>
>> Now it’s possible to have % char inside the expression, for example:
>>
>> RTE_ASSERT((sizeof(some_struct) % 64) == 0)
>>
>> Before the patch, “%" sign acts like a conversion specification beginning
>> character.
> 
> previous sample is not correct, complex expressions "RTE_ASSERT((1 + 2) == 3)"
> are working fine.
> But you are right "%" causing problem, and this patch is fixing it.
> 
> Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>

Applied to dpdk/master, thanks.

Note added into commit log.
  

Patch

diff --git a/lib/librte_eal/common/include/rte_debug.h b/lib/librte_eal/common/include/rte_debug.h
index 79b67b3ec..fbb3bb5e5 100644
--- a/lib/librte_eal/common/include/rte_debug.h
+++ b/lib/librte_eal/common/include/rte_debug.h
@@ -86,7 +86,7 @@  void rte_dump_registers(void);
 #endif
 #define	RTE_VERIFY(exp)	do {                                                  \
 	if (unlikely(!(exp)))                                                           \
-		rte_panic("line %d\tassert \"" #exp "\" failed\n", __LINE__); \
+		rte_panic("line %d\tassert \"%s\" failed\n", __LINE__, #exp); \
 } while (0)

 /*