Re: [dpdk-dev] [dpdk-stable] [PATCH v2] test/hash: solve unit test hash compilation error

[Dharmik Thakkar <Dharmik.Thakkar@arm.com>]

> On Oct 26, 2018, at 3:24 PM, Thomas Monjalon <thomas@monjalon.net> wrote:
> 
> +Cc Yipeng
> 
> 18/09/2018 21:22, Dharmik Thakkar:
>> Enable print_key_info() function compilation always.
> 
> Please see my first comment: you need to show the compilation error
> in this message. Otherwise, we don't know what you are trying
> to fix.
Sure, I will submit a new version with the compilation error message.
> 
>> Fixes: af75078fece36 ("first public release")
>> Cc: stable@dpdk.org
>> 
>> Suggested-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
>> Signed-off-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
>> Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
>> Reviewed-by: Gavin Hu <gavin.hu@arm.com>
>> ---
>> v2:
>> * Fix checkpatch coding style issue
>> * Add "Fixes:" tag
>> ---
>> test/test/test_hash.c | 24 +++++++++---------------
>> 1 file changed, 9 insertions(+), 15 deletions(-)
>> 
>> diff --git a/test/test/test_hash.c b/test/test/test_hash.c
>> index b3db9fd10547..db6442a2b101 100644
>> --- a/test/test/test_hash.c
>> +++ b/test/test/test_hash.c
>> +#define UNIT_TEST_HASH_VERBOSE	0
>> /*
>>  * Print out result of unit test hash operation.
>>  */
>> -#if defined(UNIT_TEST_HASH_VERBOSE)
>> static void print_key_info(const char *msg, const struct flow_key *key,
>> 								int32_t pos)
>> {
>> -	uint8_t *p = (uint8_t *)key;
>> -	unsigned i;
>> -
>> -	printf("%s key:0x", msg);
>> -	for (i = 0; i < sizeof(struct flow_key); i++) {
>> -		printf("%02X", p[i]);
>> +	if (UNIT_TEST_HASH_VERBOSE) {
> 
> This is very suspicious.
> Why keeping this code if it is never called?
The function ‘print_key_info’ is being called at different places and developer can set the macro, to print debug information.
> 
>> +		const uint8_t *p = (const uint8_t *)key;
>> +		unsigned int i;
>> +
>> +		printf("%s key:0x", msg);
>> +		for (i = 0; i < sizeof(struct flow_key); i++)
>> +			printf("%02X", p[i]);
>> +		printf(" @ pos %d\n", pos);
>> 	}
>> -	printf(" @ pos %d\n", pos);
>> -}
>> -#else
>> -static void print_key_info(__attribute__((unused)) const char *msg,
>> -		__attribute__((unused)) const struct flow_key *key,
>> -		__attribute__((unused)) int32_t pos)
>> -{
>> }
>> -#endif