[lttng-dev] [PATCH babeltrace 2/2] Extend test-bitfield coverage

[Mathieu Desnoyers]

----- On May 10, 2019, at 5:11 PM, Simon Marchi simon.marchi at efficios.com wrote:

> On 2019-05-10 1:47 p.m., Mathieu Desnoyers wrote:
>> test-bitfield was mainly testing various write unit size. Add
>> variations of read unit size as well.
>> 
>> Previously, the test was only covering input from a 32-bit integer.
>> Additionally test source and destination of 64-bit.
>> 
>> Change-Id: Ic2b7849140e04fe7cca3de576e31bfef8f0a03ae
> 
> Missing Signed-off-by?

Oops, yes.

> 
> LGTM, I just have some minor comments:
> 
>> +#ifndef HAS_FLS_U64
>> +static __attribute__((unused))
>> +unsigned int fls_u64(uint64_t x)
>>  {
>> -	int r = 32;
>> +	unsigned int r = 64;
>> +
>> +	if (!x)
>> +		return 0;
>> +
>> +	if (!(x & 0xFFFFFFFF00000000ULL)) {
>> +		x <<= 32;
>> +		r -= 32;
>> +	}
>> +	if (!(x & 0xFFFF000000000000ULL)) {
>> +		x <<= 16;
>> +		r -= 16;
>> +	}
>> +	if (!(x & 0xFF00000000000000ULL)) {
>> +		x <<= 8;
>> +		r -= 8;
>> +	}
>> +	if (!(x & 0xF000000000000000ULL)) {
>> +		x <<= 4;
>> +		r -= 4;
>> +	}
>> +	if (!(x & 0xC000000000000000ULL)) {
>> +		x <<= 2;
>> +		r -= 2;
>> +	}
>> +	if (!(x & 0x8000000000000000ULL)) {
>> +		x <<= 1;
>> +		r -= 1;
>> +	}
>> +	return r;
>> +}
>> +#endif
>> +
>> +#ifndef HAS_FLS_U32
>> +static __attribute__((unused))
> 
> The unused attributes, in both functions, should be unnecessary here, I suppose?

Yes, will remove.


> 
>>  void run_test(void)
>>  {
>>  	int i;
>> -	plan_tests(NR_TESTS * 2 + 6);
>> +	plan_tests(NR_TESTS * 8 + 24);
>>  
>>  	srand(time(NULL));
>>  
>> -	srcrand = 0;
>> +	srcrand_ui = 0;
>> +	srcrand_ull = 0;
>>  	run_test_unsigned();
> 
> Not a big deal (and orthogonal to the problem being addressed), but I was
> wondering, why
> the test values are passed through global variables, and not by parameter, such
> as

Historic evolution starting with a quick and dirty prototype back in 2010. I'll
do a cleanup patch on top to move this to parameter passing, which will be a patch
3 of the next patchset round.

Thanks!

Mathieu

> 
>    run_test_unsigned(0, 0);
> 
> Simon

-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com