* [lttng-dev] [PATCH v7 babeltrace 1/4] Fix: bitfield: shift undefined/implementation defined behaviors
@ 2019-05-15 21:46 Mathieu Desnoyers
2019-05-15 21:46 ` [lttng-dev] [PATCH babeltrace 2/4] Silence compiler "always false comparison" warning Mathieu Desnoyers
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Mathieu Desnoyers @ 2019-05-15 21:46 UTC (permalink / raw)
bitfield.h uses the left shift operator with a left operand which
may be negative. The C99 standard states that shifting a negative
value is undefined.
When building with -Wshift-negative-value, we get this gcc warning:
In file included from /home/smarchi/src/babeltrace/include/babeltrace/ctfser-internal.h:44:0,
from /home/smarchi/src/babeltrace/ctfser/ctfser.c:42:
/home/smarchi/src/babeltrace/include/babeltrace/ctfser-internal.h: In function ?bt_ctfser_write_unsigned_int?:
/home/smarchi/src/babeltrace/include/babeltrace/bitfield-internal.h:116:24: error: left shift of negative value [-Werror=shift-negative-value]
mask = ~((~(type) 0) << (__start % ts)); \
^
/home/smarchi/src/babeltrace/include/babeltrace/bitfield-internal.h:222:2: note: in expansion of macro ?_bt_bitfield_write_le?
_bt_bitfield_write_le(ptr, type, _start, _length, _v)
^~~~~~~~~~~~~~~~~~~~~
/home/smarchi/src/babeltrace/include/babeltrace/ctfser-internal.h:418:3: note: in expansion of macro ?bt_bitfield_write_le?
bt_bitfield_write_le(mmap_align_addr(ctfser->base_mma) +
^~~~~~~~~~~~~~~~~~~~
This boils down to the fact that the expression ~((uint8_t)0) has type
"signed int", which is used as an operand of the left shift. This is due
to the integer promotion rules of C99 (6.3.3.1):
If an int can represent all values of the original type, the value is
converted to an int; otherwise, it is converted to an unsigned int.
These are called the integer promotions. All other types are unchanged
by the integer promotions.
We also need to cast the result explicitly into the left hand
side type to deal with:
warning: large integer implicitly truncated to unsigned type [-Woverflow]
The C99 standard states that a right shift has implementation-defined
behavior when shifting a signed negative value. Add a preprocessor check
that the compiler provides the expected behavior, else provide an
alternative implementation which guarantees the intended behavior.
A preprocessor check is also added to ensure that the compiler
representation for signed values is two's complement, which is expected
by this header.
Document that this header strictly respects the C99 standard, with
the exception of its use of __typeof__.
Adapt use of _bt_piecewise_lshift in plugins/text/pretty/print.c
to the new API.
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers at efficios.com>
Acked-by: Simon Marchi <simon.marchi at efficios.com>
Change-Id: I47d2655cfe671baf0fc18813883adf7ce11dfd6a
---
Changes since v1:
- Generate compile-time error if the type argument passed to
_bt_unsigned_cast() is larger than sizeof(uint64_t), this
allows removing _bt_check_max_64bit,
- Introduce _br_fill_mask, which replaces _bt_bitwise_not,
- Clarify _bt_unsigned_cast comment,
- Expand explanation of the issue within the patch commit message.
Changes since v2:
- Fix unwanted sign extension when generating masks,
- Introduce macro helpers to clarify code:
- _bt_cast_value_to_unsigned()
- _bt_cast_value_to_unsigned_type(),
- _bt_make_mask_complement(),
- _bt_make_mask().
Changes since v3:
- Fix additional left shift undefined behavior. Identified with
-fsanitize=undefined.
- Create fallback for right shift implementation-defined behavior
if the behavior is found not to be as intended. Detect the behavior
with a preprocessor check.
- Ensure that the compiler represents signed types with two's complement
with a preprocessor check.
- Add _bt_rshift() and _bt_lshift() shift helpers to take care of
C99's undefined behaviors related to shifting signed types.
- Remove statement-expressions to allow building with -std=c99.
- Use __typeof__ instead of typeof to allow building with -std=c99.
Strictly speaking, the use of __typeof__ is specific to the compiler.
Therefore, porting to a strict ansi C compiler would require removing
those __typeof__.
- Remove use of gnu extension: replace (a ? : b) expressions by (a ? a :
b) to please compiling with -std=c99 -Wpedantic.
Changes since v4:
- Document strict C89 compliance, except for use of __typeof__
compiler-specific extension.
- Adapt use of _bt_piecewise_lshift in plugins/text/pretty/print.c
to the new API.
Changes since v5:
- Bump standard compliance from C89 to C99, because we use
C99 types.h.
Changes since v6:
- Fix wrong use of _bt_make_mask() in _bt_bitfield_write_be. Should
be _bt_make_mask_complement. Caught by testing on powerpc.
---
include/babeltrace/bitfield-internal.h | 298 +++++++++++++++++++++------------
plugins/text/pretty/print.c | 4 +-
2 files changed, 195 insertions(+), 107 deletions(-)
diff --git a/include/babeltrace/bitfield-internal.h b/include/babeltrace/bitfield-internal.h
index c5d5eccd..31b18f15 100644
--- a/include/babeltrace/bitfield-internal.h
+++ b/include/babeltrace/bitfield-internal.h
@@ -2,7 +2,7 @@
#define _BABELTRACE_BITFIELD_H
/*
- * Copyright 2010 - Mathieu Desnoyers <mathieu.desnoyers at efficios.com>
+ * Copyright 2010-2019 - Mathieu Desnoyers <mathieu.desnoyers at efficios.com>
*
* Permission is hereby granted, free of charge, to any person obtaining a copy
* of this software and associated documentation files (the "Software"), to deal
@@ -27,49 +27,145 @@
#include <babeltrace/compat/limits-internal.h> /* C99 5.2.4.2 Numerical limits */
#include <babeltrace/endian-internal.h> /* Non-standard BIG_ENDIAN, LITTLE_ENDIAN, BYTE_ORDER */
-/* We can't shift a int from 32 bit, >> 32 and << 32 on int is undefined */
-#define _bt_piecewise_rshift(_v, _shift) \
-({ \
- typeof(_v) ___v = (_v); \
- typeof(_shift) ___shift = (_shift); \
- unsigned long sb = (___shift) / (sizeof(___v) * CHAR_BIT - 1); \
- unsigned long final = (___shift) % (sizeof(___v) * CHAR_BIT - 1); \
- \
- for (; sb; sb--) \
- ___v >>= sizeof(___v) * CHAR_BIT - 1; \
- ___v >>= final; \
-})
+/*
+ * This header strictly follows the C99 standard, except for use of the
+ * compiler-specific __typeof__.
+ */
-#define _bt_piecewise_lshift(_v, _shift) \
-({ \
- typeof(_v) ___v = (_v); \
- typeof(_shift) ___shift = (_shift); \
- unsigned long sb = (___shift) / (sizeof(___v) * CHAR_BIT - 1); \
- unsigned long final = (___shift) % (sizeof(___v) * CHAR_BIT - 1); \
- \
- for (; sb; sb--) \
- ___v <<= sizeof(___v) * CHAR_BIT - 1; \
- ___v <<= final; \
-})
+/*
+ * This bitfield header requires the compiler representation of signed
+ * integers to be two's complement.
+ */
+#if (-1 != ~0)
+#error "bitfield.h requires the compiler representation of signed integers to be two's complement."
+#endif
#define _bt_is_signed_type(type) ((type) -1 < (type) 0)
/*
- * NOTE: The cast to (uint64_t) below ensures that we're not casting a
- * negative value, which is undefined in C. However, this limits the
- * maximum type size of `type` and `v` to 64-bit. The
- * _bt_check_max_64bit() is used to check that the users of this header
- * do not use types with a size greater than 64-bit.
+ * Produce a build-time error if the condition `cond` is non-zero.
+ * Evaluates as a size_t expression.
+ */
+#define _BT_BUILD_ASSERT(cond) \
+ sizeof(struct { int f:(2 * !!(cond) - 1); })
+
+/*
+ * Cast value `v` to an unsigned integer of the same size as `v`.
+ */
+#define _bt_cast_value_to_unsigned(v) \
+ (sizeof(v) == sizeof(uint8_t) ? (uint8_t) (v) : \
+ sizeof(v) == sizeof(uint16_t) ? (uint16_t) (v) : \
+ sizeof(v) == sizeof(uint32_t) ? (uint32_t) (v) : \
+ sizeof(v) == sizeof(uint64_t) ? (uint64_t) (v) : \
+ _BT_BUILD_ASSERT(sizeof(v) <= sizeof(uint64_t)))
+
+/*
+ * Cast value `v` to an unsigned integer type of the size of type `type`
+ * *without* sign-extension.
+ *
+ * The unsigned cast ensures that we're not shifting a negative value,
+ * which is undefined in C. However, this limits the maximum type size
+ * of `type` to 64-bit. Generate a compile-time error if the size of
+ * `type` is larger than 64-bit.
*/
-#define _bt_unsigned_cast(type, v) \
-({ \
- (sizeof(v) < sizeof(type)) ? \
- ((type) (v)) & ((type) (~(~(uint64_t) 0 << (sizeof(v) * CHAR_BIT)))) : \
- (type) (v); \
-})
+#define _bt_cast_value_to_unsigned_type(type, v) \
+ (sizeof(type) == sizeof(uint8_t) ? \
+ (uint8_t) _bt_cast_value_to_unsigned(v) : \
+ sizeof(type) == sizeof(uint16_t) ? \
+ (uint16_t) _bt_cast_value_to_unsigned(v) : \
+ sizeof(type) == sizeof(uint32_t) ? \
+ (uint32_t) _bt_cast_value_to_unsigned(v) : \
+ sizeof(type) == sizeof(uint64_t) ? \
+ (uint64_t) _bt_cast_value_to_unsigned(v) : \
+ _BT_BUILD_ASSERT(sizeof(v) <= sizeof(uint64_t)))
-#define _bt_check_max_64bit(type) \
- char _max_64bit_assertion[sizeof(type) <= sizeof(uint64_t) ? 1 : -1] __attribute__((unused))
+/*
+ * _bt_fill_mask evaluates to a "type" integer with all bits set.
+ */
+#define _bt_fill_mask(type) ((type) ~(type) 0)
+
+/*
+ * Left shift a value `v` of `shift` bits.
+ *
+ * The type of `v` can be signed or unsigned integer.
+ * The value of `shift` must be less than the size of `v` (in bits),
+ * otherwise the behavior is undefined.
+ * Evaluates to the result of the shift operation.
+ *
+ * According to the C99 standard, left shift of a left hand-side signed
+ * type is undefined if it has a negative value or if the result cannot
+ * be represented in the result type. This bitfield header discards the
+ * bits that are left-shifted beyond the result type representation,
+ * which is the behavior of an unsigned type left shift operation.
+ * Therefore, always perform left shift on an unsigned type.
+ */
+#define _bt_lshift(v, shift) \
+ ((__typeof__(v)) (_bt_cast_value_to_unsigned(v) << (shift)))
+
+/*
+ * Generate a mask of type `type` with the `length` least significant bits
+ * cleared, and the most significant bits set.
+ */
+#define _bt_make_mask_complement(type, length) \
+ _bt_lshift(_bt_fill_mask(type), length)
+
+/*
+ * Generate a mask of type `type` with the `length` least significant bits
+ * set, and the most significant bits cleared.
+ */
+#define _bt_make_mask(type, length) \
+ ((type) ~_bt_make_mask_complement(type, length))
+
+/*
+ * Right shift a value `v` of `shift` bits.
+ *
+ * The type of `v` can be signed or unsigned integer.
+ * The value of `shift` must be less than the size of `v` (in bits),
+ * otherwise the behavior is undefined.
+ * Evaluates to the result of the shift operation.
+ *
+ * According to the C99 standard, right shift of a left hand-side signed
+ * type which has a negative value is implementation defined. This
+ * bitfield header relies on the right shift implementation carrying the
+ * sign bit. If the compiler implementation has a different behavior,
+ * emulate carrying the sign bit.
+ */
+#if ((-1 >> 1) == -1)
+#define _bt_rshift(v, shift) ((v) >> (shift))
+#else
+#define _bt_rshift(v, shift) \
+ ((__typeof__(v)) ((_bt_cast_value_to_unsigned(v) >> (shift)) | \
+ ((v) < 0 ? _bt_make_mask_complement(__typeof__(v), \
+ sizeof(v) * CHAR_BIT - (shift)) : 0)))
+#endif
+
+/*
+ * Right shift a signed or unsigned integer with `_shift` value being an
+ * arbitrary number of bits. `_v` is modified by this macro.
+ */
+#define _bt_piecewise_rshift(_v, _shift) \
+do { \
+ unsigned long _sb = (_shift) / (sizeof(_v) * CHAR_BIT - 1); \
+ unsigned long _final = (_shift) % (sizeof(_v) * CHAR_BIT - 1); \
+ \
+ for (; _sb; _sb--) \
+ (_v) = _bt_rshift(_v, sizeof(_v) * CHAR_BIT - 1); \
+ (_v) = _bt_rshift(_v, _final); \
+} while (0)
+
+/*
+ * Left shift a signed or unsigned integer with `_shift` value being an
+ * arbitrary number of bits. `_v` is modified by this macro.
+ */
+#define _bt_piecewise_lshift(_v, _shift) \
+do { \
+ unsigned long _sb = (_shift) / (sizeof(_v) * CHAR_BIT - 1); \
+ unsigned long _final = (_shift) % (sizeof(_v) * CHAR_BIT - 1); \
+ \
+ for (; _sb; _sb--) \
+ (_v) = _bt_lshift(_v, sizeof(_v) * CHAR_BIT - 1); \
+ (_v) = _bt_lshift(_v, _final); \
+} while (0)
/*
* bt_bitfield_write - write integer to a bitfield in native endianness
@@ -91,7 +187,7 @@
#define _bt_bitfield_write_le(_ptr, type, _start, _length, _v) \
do { \
- typeof(_v) __v = (_v); \
+ __typeof__(_v) __v = (_v); \
type *__ptr = (void *) (_ptr); \
unsigned long __start = (_start), __length = (_length); \
type mask, cmask; \
@@ -108,15 +204,15 @@ do { \
\
/* Trim v high bits */ \
if (__length < sizeof(__v) * CHAR_BIT) \
- __v &= ~((~(typeof(__v)) 0) << __length); \
+ __v &= _bt_make_mask(__typeof__(__v), __length); \
\
/* We can now append v with a simple "or", shift it piece-wise */ \
this_unit = start_unit; \
if (start_unit == end_unit - 1) { \
- mask = ~((~(type) 0) << (__start % ts)); \
+ mask = _bt_make_mask(type, __start % ts); \
if (end % ts) \
- mask |= (~(type) 0) << (end % ts); \
- cmask = (type) __v << (__start % ts); \
+ mask |= _bt_make_mask_complement(type, end % ts); \
+ cmask = _bt_lshift((type) (__v), __start % ts); \
cmask &= ~mask; \
__ptr[this_unit] &= mask; \
__ptr[this_unit] |= cmask; \
@@ -124,22 +220,22 @@ do { \
} \
if (__start % ts) { \
cshift = __start % ts; \
- mask = ~((~(type) 0) << cshift); \
- cmask = (type) __v << cshift; \
+ mask = _bt_make_mask(type, cshift); \
+ cmask = _bt_lshift((type) (__v), cshift); \
cmask &= ~mask; \
__ptr[this_unit] &= mask; \
__ptr[this_unit] |= cmask; \
- __v = _bt_piecewise_rshift(__v, ts - cshift); \
+ _bt_piecewise_rshift(__v, ts - cshift); \
__start += ts - cshift; \
this_unit++; \
} \
for (; this_unit < end_unit - 1; this_unit++) { \
__ptr[this_unit] = (type) __v; \
- __v = _bt_piecewise_rshift(__v, ts); \
+ _bt_piecewise_rshift(__v, ts); \
__start += ts; \
} \
if (end % ts) { \
- mask = (~(type) 0) << (end % ts); \
+ mask = _bt_make_mask_complement(type, end % ts); \
cmask = (type) __v; \
cmask &= ~mask; \
__ptr[this_unit] &= mask; \
@@ -150,7 +246,7 @@ do { \
#define _bt_bitfield_write_be(_ptr, type, _start, _length, _v) \
do { \
- typeof(_v) __v = (_v); \
+ __typeof__(_v) __v = (_v); \
type *__ptr = (void *) (_ptr); \
unsigned long __start = (_start), __length = (_length); \
type mask, cmask; \
@@ -167,15 +263,15 @@ do { \
\
/* Trim v high bits */ \
if (__length < sizeof(__v) * CHAR_BIT) \
- __v &= ~((~(typeof(__v)) 0) << __length); \
+ __v &= _bt_make_mask(__typeof__(__v), __length); \
\
/* We can now append v with a simple "or", shift it piece-wise */ \
this_unit = end_unit - 1; \
if (start_unit == end_unit - 1) { \
- mask = ~((~(type) 0) << ((ts - (end % ts)) % ts)); \
+ mask = _bt_make_mask(type, (ts - (end % ts)) % ts); \
if (__start % ts) \
- mask |= (~((type) 0)) << (ts - (__start % ts)); \
- cmask = (type) __v << ((ts - (end % ts)) % ts); \
+ mask |= _bt_make_mask_complement(type, ts - (__start % ts)); \
+ cmask = _bt_lshift((type) (__v), (ts - (end % ts)) % ts); \
cmask &= ~mask; \
__ptr[this_unit] &= mask; \
__ptr[this_unit] |= cmask; \
@@ -183,22 +279,22 @@ do { \
} \
if (end % ts) { \
cshift = end % ts; \
- mask = ~((~(type) 0) << (ts - cshift)); \
- cmask = (type) __v << (ts - cshift); \
+ mask = _bt_make_mask(type, ts - cshift); \
+ cmask = _bt_lshift((type) (__v), ts - cshift); \
cmask &= ~mask; \
__ptr[this_unit] &= mask; \
__ptr[this_unit] |= cmask; \
- __v = _bt_piecewise_rshift(__v, cshift); \
+ _bt_piecewise_rshift(__v, cshift); \
end -= cshift; \
this_unit--; \
} \
for (; (long) this_unit >= (long) start_unit + 1; this_unit--) { \
__ptr[this_unit] = (type) __v; \
- __v = _bt_piecewise_rshift(__v, ts); \
+ _bt_piecewise_rshift(__v, ts); \
end -= ts; \
} \
if (__start % ts) { \
- mask = (~(type) 0) << (ts - (__start % ts)); \
+ mask = _bt_make_mask_complement(type, ts - (__start % ts)); \
cmask = (type) __v; \
cmask &= ~mask; \
__ptr[this_unit] &= mask; \
@@ -243,8 +339,8 @@ do { \
#define _bt_bitfield_read_le(_ptr, type, _start, _length, _vptr) \
do { \
- typeof(*(_vptr)) *__vptr = (_vptr); \
- typeof(*__vptr) __v; \
+ __typeof__(*(_vptr)) *__vptr = (_vptr); \
+ __typeof__(*__vptr) __v; \
type *__ptr = (void *) (_ptr); \
unsigned long __start = (_start), __length = (_length); \
type mask, cmask; \
@@ -252,10 +348,6 @@ do { \
unsigned long start_unit, end_unit, this_unit; \
unsigned long end, cshift; /* cshift is "complement shift" */ \
\
- { _bt_check_max_64bit(type); } \
- { _bt_check_max_64bit(typeof(*_vptr)); } \
- { _bt_check_max_64bit(typeof(*_ptr)); } \
- \
if (!__length) { \
*__vptr = 0; \
break; \
@@ -266,56 +358,56 @@ do { \
end_unit = (end + (ts - 1)) / ts; \
\
this_unit = end_unit - 1; \
- if (_bt_is_signed_type(typeof(__v)) \
- && (__ptr[this_unit] & ((type) 1 << ((end % ts ? : ts) - 1)))) \
- __v = ~(typeof(__v)) 0; \
+ if (_bt_is_signed_type(__typeof__(__v)) \
+ && (__ptr[this_unit] & _bt_lshift((type) 1, (end % ts ? end % ts : ts) - 1))) \
+ __v = ~(__typeof__(__v)) 0; \
else \
__v = 0; \
if (start_unit == end_unit - 1) { \
cmask = __ptr[this_unit]; \
- cmask >>= (__start % ts); \
+ cmask = _bt_rshift(cmask, __start % ts); \
if ((end - __start) % ts) { \
- mask = ~((~(type) 0) << (end - __start)); \
+ mask = _bt_make_mask(type, end - __start); \
cmask &= mask; \
} \
- __v = _bt_piecewise_lshift(__v, end - __start); \
- __v |= _bt_unsigned_cast(typeof(__v), cmask); \
+ _bt_piecewise_lshift(__v, end - __start); \
+ __v |= _bt_cast_value_to_unsigned_type(__typeof__(__v), cmask); \
*__vptr = __v; \
break; \
} \
if (end % ts) { \
cshift = end % ts; \
- mask = ~((~(type) 0) << cshift); \
+ mask = _bt_make_mask(type, cshift); \
cmask = __ptr[this_unit]; \
cmask &= mask; \
- __v = _bt_piecewise_lshift(__v, cshift); \
- __v |= _bt_unsigned_cast(typeof(__v), cmask); \
+ _bt_piecewise_lshift(__v, cshift); \
+ __v |= _bt_cast_value_to_unsigned_type(__typeof__(__v), cmask); \
end -= cshift; \
this_unit--; \
} \
for (; (long) this_unit >= (long) start_unit + 1; this_unit--) { \
- __v = _bt_piecewise_lshift(__v, ts); \
- __v |= _bt_unsigned_cast(typeof(__v), __ptr[this_unit]);\
+ _bt_piecewise_lshift(__v, ts); \
+ __v |= _bt_cast_value_to_unsigned_type(__typeof__(__v), __ptr[this_unit]); \
end -= ts; \
} \
if (__start % ts) { \
- mask = ~((~(type) 0) << (ts - (__start % ts))); \
+ mask = _bt_make_mask(type, ts - (__start % ts)); \
cmask = __ptr[this_unit]; \
- cmask >>= (__start % ts); \
+ cmask = _bt_rshift(cmask, __start % ts); \
cmask &= mask; \
- __v = _bt_piecewise_lshift(__v, ts - (__start % ts)); \
- __v |= _bt_unsigned_cast(typeof(__v), cmask); \
+ _bt_piecewise_lshift(__v, ts - (__start % ts)); \
+ __v |= _bt_cast_value_to_unsigned_type(__typeof__(__v), cmask); \
} else { \
- __v = _bt_piecewise_lshift(__v, ts); \
- __v |= _bt_unsigned_cast(typeof(__v), __ptr[this_unit]);\
+ _bt_piecewise_lshift(__v, ts); \
+ __v |= _bt_cast_value_to_unsigned_type(__typeof__(__v), __ptr[this_unit]); \
} \
*__vptr = __v; \
} while (0)
#define _bt_bitfield_read_be(_ptr, type, _start, _length, _vptr) \
do { \
- typeof(*(_vptr)) *__vptr = (_vptr); \
- typeof(*__vptr) __v; \
+ __typeof__(*(_vptr)) *__vptr = (_vptr); \
+ __typeof__(*__vptr) __v; \
type *__ptr = (void *) (_ptr); \
unsigned long __start = (_start), __length = (_length); \
type mask, cmask; \
@@ -323,10 +415,6 @@ do { \
unsigned long start_unit, end_unit, this_unit; \
unsigned long end, cshift; /* cshift is "complement shift" */ \
\
- { _bt_check_max_64bit(type); } \
- { _bt_check_max_64bit(typeof(*_vptr)); } \
- { _bt_check_max_64bit(typeof(*_ptr)); } \
- \
if (!__length) { \
*__vptr = 0; \
break; \
@@ -337,48 +425,48 @@ do { \
end_unit = (end + (ts - 1)) / ts; \
\
this_unit = start_unit; \
- if (_bt_is_signed_type(typeof(__v)) \
- && (__ptr[this_unit] & ((type) 1 << (ts - (__start % ts) - 1)))) \
- __v = ~(typeof(__v)) 0; \
+ if (_bt_is_signed_type(__typeof__(__v)) \
+ && (__ptr[this_unit] & _bt_lshift((type) 1, ts - (__start % ts) - 1))) \
+ __v = ~(__typeof__(__v)) 0; \
else \
__v = 0; \
if (start_unit == end_unit - 1) { \
cmask = __ptr[this_unit]; \
- cmask >>= (ts - (end % ts)) % ts; \
+ cmask = _bt_rshift(cmask, (ts - (end % ts)) % ts); \
if ((end - __start) % ts) { \
- mask = ~((~(type) 0) << (end - __start)); \
+ mask = _bt_make_mask(type, end - __start); \
cmask &= mask; \
} \
- __v = _bt_piecewise_lshift(__v, end - __start); \
- __v |= _bt_unsigned_cast(typeof(__v), cmask); \
+ _bt_piecewise_lshift(__v, end - __start); \
+ __v |= _bt_cast_value_to_unsigned_type(__typeof__(__v), cmask); \
*__vptr = __v; \
break; \
} \
if (__start % ts) { \
cshift = __start % ts; \
- mask = ~((~(type) 0) << (ts - cshift)); \
+ mask = _bt_make_mask(type, ts - cshift); \
cmask = __ptr[this_unit]; \
cmask &= mask; \
- __v = _bt_piecewise_lshift(__v, ts - cshift); \
- __v |= _bt_unsigned_cast(typeof(__v), cmask); \
+ _bt_piecewise_lshift(__v, ts - cshift); \
+ __v |= _bt_cast_value_to_unsigned_type(__typeof__(__v), cmask); \
__start += ts - cshift; \
this_unit++; \
} \
for (; this_unit < end_unit - 1; this_unit++) { \
- __v = _bt_piecewise_lshift(__v, ts); \
- __v |= _bt_unsigned_cast(typeof(__v), __ptr[this_unit]);\
+ _bt_piecewise_lshift(__v, ts); \
+ __v |= _bt_cast_value_to_unsigned_type(__typeof__(__v), __ptr[this_unit]); \
__start += ts; \
} \
if (end % ts) { \
- mask = ~((~(type) 0) << (end % ts)); \
+ mask = _bt_make_mask(type, end % ts); \
cmask = __ptr[this_unit]; \
- cmask >>= ts - (end % ts); \
+ cmask = _bt_rshift(cmask, ts - (end % ts)); \
cmask &= mask; \
- __v = _bt_piecewise_lshift(__v, end % ts); \
- __v |= _bt_unsigned_cast(typeof(__v), cmask); \
+ _bt_piecewise_lshift(__v, end % ts); \
+ __v |= _bt_cast_value_to_unsigned_type(__typeof__(__v), cmask); \
} else { \
- __v = _bt_piecewise_lshift(__v, ts); \
- __v |= _bt_unsigned_cast(typeof(__v), __ptr[this_unit]);\
+ _bt_piecewise_lshift(__v, ts); \
+ __v |= _bt_cast_value_to_unsigned_type(__typeof__(__v), __ptr[this_unit]); \
} \
*__vptr = __v; \
} while (0)
diff --git a/plugins/text/pretty/print.c b/plugins/text/pretty/print.c
index 95189d5e..2d5ac0d2 100644
--- a/plugins/text/pretty/print.c
+++ b/plugins/text/pretty/print.c
@@ -546,10 +546,10 @@ int print_integer(struct pretty_component *pretty,
len = bt_field_class_integer_get_field_value_range(int_fc);
g_string_append(pretty->string, "0b");
- v.u = _bt_piecewise_lshift(v.u, 64 - len);
+ _bt_piecewise_lshift(v.u, 64 - len);
for (bitnr = 0; bitnr < len; bitnr++) {
g_string_append_printf(pretty->string, "%u", (v.u & (1ULL << 63)) ? 1 : 0);
- v.u = _bt_piecewise_lshift(v.u, 1);
+ _bt_piecewise_lshift(v.u, 1);
}
break;
}
--
2.11.0
^ permalink raw reply [flat|nested] 5+ messages in thread
* [lttng-dev] [PATCH babeltrace 2/4] Silence compiler "always false comparison" warning
2019-05-15 21:46 [lttng-dev] [PATCH v7 babeltrace 1/4] Fix: bitfield: shift undefined/implementation defined behaviors Mathieu Desnoyers
@ 2019-05-15 21:46 ` Mathieu Desnoyers
2019-05-15 21:46 ` [lttng-dev] [PATCH babeltrace 3/4] Extend test_bitfield coverage Mathieu Desnoyers
2019-05-15 21:46 ` [lttng-dev] [PATCH babeltrace 4/4] Cleanup: test_bitfield: pass source integers as parameters Mathieu Desnoyers
2 siblings, 0 replies; 5+ messages in thread
From: Mathieu Desnoyers @ 2019-05-15 21:46 UTC (permalink / raw)
Compiling the bitfield test with gcc -Wextra generates those warnings:
../../include/babeltrace/bitfield-internal.h:38:45: warning: comparison of unsigned expression < 0 is always false [-Wtype-limits]
#define _bt_is_signed_type(type) ((type) -1 < (type) 0)
This is the intent of the macro. Disable compiler warnings around use of
that macro.
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers at efficios.com>
Change-Id: I2dd980e11ebd6cd37ae71b013925a6fca5d7b483
---
include/babeltrace/bitfield-internal.h | 34 ++++++++++++++++++++++++++++++++--
1 file changed, 32 insertions(+), 2 deletions(-)
diff --git a/include/babeltrace/bitfield-internal.h b/include/babeltrace/bitfield-internal.h
index 31b18f15..01d00c13 100644
--- a/include/babeltrace/bitfield-internal.h
+++ b/include/babeltrace/bitfield-internal.h
@@ -40,6 +40,26 @@
#error "bitfield.h requires the compiler representation of signed integers to be two's complement."
#endif
+/*
+ * _bt_is_signed_type() willingly generates comparison of unsigned
+ * expression < 0, which is always false. Silence compiler warnings.
+ */
+#ifdef __GNUC__
+# define _BT_DIAG_PUSH _Pragma("GCC diagnostic push")
+# define _BT_DIAG_POP _Pragma("GCC diagnostic pop")
+
+# define _BT_DIAG_STRINGIFY_1(x) #x
+# define _BT_DIAG_STRINGIFY(x) _BT_DIAG_STRINGIFY_1(x)
+
+# define _BT_DIAG_IGNORE(option) \
+ _Pragma(_BT_DIAG_STRINGIFY(GCC diagnostic ignored option))
+# define _BT_DIAG_IGNORE_TYPE_LIMITS _BT_DIAG_IGNORE("-Wtype-limits")
+#else
+# define _BT_DIAG_PUSH
+# define _BT_DIAG_POP
+# define _BT_DIAG_IGNORE
+#endif
+
#define _bt_is_signed_type(type) ((type) -1 < (type) 0)
/*
@@ -347,6 +367,7 @@ do { \
unsigned long ts = sizeof(type) * CHAR_BIT; /* type size */ \
unsigned long start_unit, end_unit, this_unit; \
unsigned long end, cshift; /* cshift is "complement shift" */ \
+ int is_signed_type; \
\
if (!__length) { \
*__vptr = 0; \
@@ -358,7 +379,11 @@ do { \
end_unit = (end + (ts - 1)) / ts; \
\
this_unit = end_unit - 1; \
- if (_bt_is_signed_type(__typeof__(__v)) \
+ _BT_DIAG_PUSH \
+ _BT_DIAG_IGNORE_TYPE_LIMITS \
+ is_signed_type = _bt_is_signed_type(__typeof__(__v)); \
+ _BT_DIAG_POP \
+ if (is_signed_type \
&& (__ptr[this_unit] & _bt_lshift((type) 1, (end % ts ? end % ts : ts) - 1))) \
__v = ~(__typeof__(__v)) 0; \
else \
@@ -414,6 +439,7 @@ do { \
unsigned long ts = sizeof(type) * CHAR_BIT; /* type size */ \
unsigned long start_unit, end_unit, this_unit; \
unsigned long end, cshift; /* cshift is "complement shift" */ \
+ int is_signed_type; \
\
if (!__length) { \
*__vptr = 0; \
@@ -425,7 +451,11 @@ do { \
end_unit = (end + (ts - 1)) / ts; \
\
this_unit = start_unit; \
- if (_bt_is_signed_type(__typeof__(__v)) \
+ _BT_DIAG_PUSH \
+ _BT_DIAG_IGNORE_TYPE_LIMITS \
+ is_signed_type = _bt_is_signed_type(__typeof__(__v)); \
+ _BT_DIAG_POP \
+ if (is_signed_type \
&& (__ptr[this_unit] & _bt_lshift((type) 1, ts - (__start % ts) - 1))) \
__v = ~(__typeof__(__v)) 0; \
else \
--
2.11.0
^ permalink raw reply [flat|nested] 5+ messages in thread
* [lttng-dev] [PATCH babeltrace 3/4] Extend test_bitfield coverage
2019-05-15 21:46 [lttng-dev] [PATCH v7 babeltrace 1/4] Fix: bitfield: shift undefined/implementation defined behaviors Mathieu Desnoyers
2019-05-15 21:46 ` [lttng-dev] [PATCH babeltrace 2/4] Silence compiler "always false comparison" warning Mathieu Desnoyers
@ 2019-05-15 21:46 ` Mathieu Desnoyers
2019-05-15 21:46 ` [lttng-dev] [PATCH babeltrace 4/4] Cleanup: test_bitfield: pass source integers as parameters Mathieu Desnoyers
2 siblings, 0 replies; 5+ messages in thread
From: Mathieu Desnoyers @ 2019-05-15 21:46 UTC (permalink / raw)
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.
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers at efficios.com>
Change-Id: Ic2b7849140e04fe7cca3de576e31bfef8f0a03ae
---
tests/lib/test_bitfield.c | 562 ++++++++++++++++++++++++++++++++++++++++------
1 file changed, 491 insertions(+), 71 deletions(-)
diff --git a/tests/lib/test_bitfield.c b/tests/lib/test_bitfield.c
index efbbc30e..59bf89fd 100644
--- a/tests/lib/test_bitfield.c
+++ b/tests/lib/test_bitfield.c
@@ -3,7 +3,7 @@
*
* BabelTrace - bitfield test program
*
- * Copyright 2010 - Mathieu Desnoyers <mathieu.desnoyers at efficios.com>
+ * Copyright 2010-2019 - Mathieu Desnoyers <mathieu.desnoyers at efficios.com>
*
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
@@ -40,39 +40,109 @@ void fct(void)
/* Test array size, in bytes */
#define TEST_LEN 128
#define NR_TESTS 10
-#define SIGNED_TEST_DESC_FMT_STR "Writing and reading back 0x%X, signed"
-#define UNSIGNED_TEST_DESC_FMT_STR "Writing and reading back 0x%X, unsigned"
+#define SIGNED_INT_READ_TEST_DESC_FMT_STR "Writing and reading back 0x%X, signed int dest, varying read unit size"
+#define SIGNED_INT_WRITE_TEST_DESC_FMT_STR "Writing and reading back 0x%X, signed int source, varying write unit size"
+#define SIGNED_LONG_LONG_READ_TEST_DESC_FMT_STR "Writing and reading back 0x%llX, signed long long dest, varying read unit size"
+#define SIGNED_LONG_LONG_WRITE_TEST_DESC_FMT_STR "Writing and reading back 0x%llX, signed long long source, varying write unit size"
+#define UNSIGNED_INT_READ_TEST_DESC_FMT_STR "Writing and reading back 0x%X, unsigned int dest, varying read unit size"
+#define UNSIGNED_INT_WRITE_TEST_DESC_FMT_STR "Writing and reading back 0x%X, unsigned int source, varying write unit size"
+#define UNSIGNED_LONG_LONG_READ_TEST_DESC_FMT_STR "Writing and reading back 0x%llX, unsigned long long dest, varying read unit size"
+#define UNSIGNED_LONG_LONG_WRITE_TEST_DESC_FMT_STR "Writing and reading back 0x%llX, unsigned long long source, varying write unit size"
#define DIAG_FMT_STR "Failed reading value written \"%s\"-wise, with start=%i" \
" and length=%i. Read %llX"
-unsigned int srcrand;
+unsigned int srcrand_ui;
+unsigned long long srcrand_ull;
+/*
+ * fls: returns the position of the most significant bit.
+ * Returns 0 if no bit is set, else returns the position of the most
+ * significant bit (from 1 to 32 on 32-bit, from 1 to 64 on 64-bit).
+ */
#if defined(__i386) || defined(__x86_64)
-
-static inline int fls(int x)
+static
+unsigned int fls_u32(uint32_t x)
{
int r;
- asm("bsrl %1,%0\n\t"
- "cmovzl %2,%0"
- : "=&r" (r) : "rm" (x), "rm" (-1));
+
+ __asm__ ("bsrl %1,%0\n\t"
+ "jnz 1f\n\t"
+ "movl $-1,%0\n\t"
+ "1:\n\t"
+ : "=r" (r) : "rm" (x));
return r + 1;
}
-
+#define HAS_FLS_U32
#elif defined(__PPC__)
-
-static __inline__ int fls(unsigned int x)
+static
+unsigned int fls_u32(uint32_t x)
{
int lz;
- asm ("cntlzw %0,%1" : "=r" (lz) : "r" (x));
+ __asm__ ("cntlzw %0,%1" : "=r" (lz) : "r" (x));
return 32 - lz;
}
+#define HAS_FLS_U32
+#endif
+
+#if defined(__x86_64)
+static
+unsigned int fls_u64(uint64_t x)
+{
+ long r;
-#else
+ __asm__ ("bsrq %1,%0\n\t"
+ "jnz 1f\n\t"
+ "movq $-1,%0\n\t"
+ "1:\n\t"
+ : "=r" (r) : "rm" (x));
+ return r + 1;
+}
+#define HAS_FLS_U64
+#endif
+
+#ifndef HAS_FLS_U64
+static
+unsigned int fls_u64(uint64_t x)
+{
+ 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
-static int fls(unsigned int x)
+#ifndef HAS_FLS_U32
+static
+unsigned int fls_u32(uint32_t x)
{
- int r = 32;
+ unsigned int r = 32;
if (!x)
return 0;
@@ -98,7 +168,6 @@ static int fls(unsigned int x)
}
return r;
}
-
#endif
#define print_byte_array(c, len) \
@@ -133,9 +202,9 @@ do { \
(val) != (ref); \
})
-void run_test_unsigned(void)
+void run_test_unsigned_write(void)
{
- unsigned int src, nrbits;
+ unsigned int src_ui, nrbits_ui, nrbits_ull;
union {
unsigned char c[TEST_LEN];
unsigned short s[TEST_LEN/sizeof(unsigned short)];
@@ -143,62 +212,229 @@ void run_test_unsigned(void)
unsigned long l[TEST_LEN/sizeof(unsigned long)];
unsigned long long ll[TEST_LEN/sizeof(unsigned long long)];
} target;
- unsigned long long readval;
+ unsigned long long src_ull, readval;
unsigned int s, l;
- src = srcrand;
- nrbits = fls(src);
+ src_ui = srcrand_ui;
+ nrbits_ui = fls_u32(src_ui);
+ /* Write from unsigned integer src input. */
for (s = 0; s < CHAR_BIT * TEST_LEN; s++) {
- for (l = nrbits; l < (CHAR_BIT * TEST_LEN) - s; l++) {
+ for (l = nrbits_ui; l < (CHAR_BIT * TEST_LEN) - s; l++) {
init_byte_array(target.c, TEST_LEN, 0xFF);
- bt_bitfield_write(target.c, unsigned char, s, l, src);
+ bt_bitfield_write(target.c, unsigned char, s, l, src_ui);
bt_bitfield_read(target.c, unsigned char, s, l, &readval);
- if (check_result(src, readval, target.c, unsigned char,
- s, l, UNSIGNED_TEST_DESC_FMT_STR)) {
+ if (check_result(src_ui, readval, target.c, unsigned char,
+ s, l, UNSIGNED_INT_WRITE_TEST_DESC_FMT_STR)) {
return;
}
init_byte_array(target.c, TEST_LEN, 0xFF);
- bt_bitfield_write(target.s, unsigned short, s, l, src);
+ bt_bitfield_write(target.s, unsigned short, s, l, src_ui);
bt_bitfield_read(target.c, unsigned char, s, l, &readval);
- if (check_result(src, readval, target.c, unsigned short,
- s, l, UNSIGNED_TEST_DESC_FMT_STR)) {
+ if (check_result(src_ui, readval, target.c, unsigned short,
+ s, l, UNSIGNED_INT_WRITE_TEST_DESC_FMT_STR)) {
return;
}
init_byte_array(target.c, TEST_LEN, 0xFF);
- bt_bitfield_write(target.i, unsigned int, s, l, src);
+ bt_bitfield_write(target.i, unsigned int, s, l, src_ui);
bt_bitfield_read(target.c, unsigned char, s, l, &readval);
- if (check_result(src, readval, target.c, unsigned int,
- s, l, UNSIGNED_TEST_DESC_FMT_STR)) {
+ if (check_result(src_ui, readval, target.c, unsigned int,
+ s, l, UNSIGNED_INT_WRITE_TEST_DESC_FMT_STR)) {
return;
}
init_byte_array(target.c, TEST_LEN, 0xFF);
- bt_bitfield_write(target.l, unsigned long, s, l, src);
+ bt_bitfield_write(target.l, unsigned long, s, l, src_ui);
bt_bitfield_read(target.c, unsigned char, s, l, &readval);
- if (check_result(src, readval, target.c, unsigned long,
- s, l, UNSIGNED_TEST_DESC_FMT_STR)) {
+ if (check_result(src_ui, readval, target.c, unsigned long,
+ s, l, UNSIGNED_INT_WRITE_TEST_DESC_FMT_STR)) {
return;
}
init_byte_array(target.c, TEST_LEN, 0xFF);
- bt_bitfield_write(target.ll, unsigned long long, s, l, src);
+ bt_bitfield_write(target.ll, unsigned long long, s, l, src_ui);
bt_bitfield_read(target.c, unsigned char, s, l, &readval);
- if (check_result(src, readval, target.c, unsigned long long,
- s, l, UNSIGNED_TEST_DESC_FMT_STR)) {
+ if (check_result(src_ui, readval, target.c, unsigned long long,
+ s, l, UNSIGNED_INT_WRITE_TEST_DESC_FMT_STR)) {
return;
}
}
}
+ pass(UNSIGNED_INT_WRITE_TEST_DESC_FMT_STR, src_ui);
+
+ src_ull = srcrand_ull;
+ nrbits_ull = fls_u64(src_ull);
+
+ /* Write from unsigned long long src input. */
+ for (s = 0; s < CHAR_BIT * TEST_LEN; s++) {
+ for (l = nrbits_ull; l < (CHAR_BIT * TEST_LEN) - s; l++) {
+ init_byte_array(target.c, TEST_LEN, 0xFF);
+ bt_bitfield_write(target.c, unsigned char, s, l, src_ull);
+ bt_bitfield_read(target.c, unsigned char, s, l, &readval);
+ if (check_result(src_ull, readval, target.c, unsigned char,
+ s, l, UNSIGNED_LONG_LONG_WRITE_TEST_DESC_FMT_STR)) {
+ return;
+ }
+
+ init_byte_array(target.c, TEST_LEN, 0xFF);
+ bt_bitfield_write(target.s, unsigned short, s, l, src_ull);
+ bt_bitfield_read(target.c, unsigned char, s, l, &readval);
+ if (check_result(src_ull, readval, target.c, unsigned short,
+ s, l, UNSIGNED_LONG_LONG_WRITE_TEST_DESC_FMT_STR)) {
+ return;
+ }
+
+ init_byte_array(target.c, TEST_LEN, 0xFF);
+ bt_bitfield_write(target.i, unsigned int, s, l, src_ull);
+ bt_bitfield_read(target.c, unsigned char, s, l, &readval);
+ if (check_result(src_ull, readval, target.c, unsigned int,
+ s, l, UNSIGNED_LONG_LONG_WRITE_TEST_DESC_FMT_STR)) {
+ return;
+ }
- pass(UNSIGNED_TEST_DESC_FMT_STR, src);
+ init_byte_array(target.c, TEST_LEN, 0xFF);
+ bt_bitfield_write(target.l, unsigned long, s, l, src_ull);
+ bt_bitfield_read(target.c, unsigned char, s, l, &readval);
+ if (check_result(src_ull, readval, target.c, unsigned long,
+ s, l, UNSIGNED_LONG_LONG_WRITE_TEST_DESC_FMT_STR)) {
+ return;
+ }
+
+ init_byte_array(target.c, TEST_LEN, 0xFF);
+ bt_bitfield_write(target.ll, unsigned long long, s, l, src_ull);
+ bt_bitfield_read(target.c, unsigned char, s, l, &readval);
+ if (check_result(src_ull, readval, target.c, unsigned long long,
+ s, l, UNSIGNED_LONG_LONG_WRITE_TEST_DESC_FMT_STR)) {
+ return;
+ }
+ }
+ }
+ pass(UNSIGNED_LONG_LONG_WRITE_TEST_DESC_FMT_STR, src_ull);
}
-void run_test_signed(void)
+void run_test_unsigned_read(void)
{
- int src, nrbits;
+ unsigned int src_ui, nrbits_ui, nrbits_ull, readval_ui;
+ union {
+ unsigned char c[TEST_LEN];
+ unsigned short s[TEST_LEN/sizeof(unsigned short)];
+ unsigned int i[TEST_LEN/sizeof(unsigned int)];
+ unsigned long l[TEST_LEN/sizeof(unsigned long)];
+ unsigned long long ll[TEST_LEN/sizeof(unsigned long long)];
+ } target;
+ unsigned long long src_ull, readval_ull;
+ unsigned int s, l;
+
+ src_ui = srcrand_ui;
+ nrbits_ui = fls_u32(src_ui);
+
+ /* Read to unsigned integer readval output. */
+ for (s = 0; s < CHAR_BIT * TEST_LEN; s++) {
+ for (l = nrbits_ui; l < (CHAR_BIT * TEST_LEN) - s; l++) {
+ init_byte_array(target.c, TEST_LEN, 0xFF);
+ bt_bitfield_write(target.c, unsigned char, s, l, src_ui);
+ bt_bitfield_read(target.c, unsigned char, s, l, &readval_ui);
+ if (check_result(src_ui, readval_ui, target.c, unsigned char,
+ s, l, UNSIGNED_INT_READ_TEST_DESC_FMT_STR)) {
+ return;
+ }
+
+ init_byte_array(target.c, TEST_LEN, 0xFF);
+ bt_bitfield_write(target.s, unsigned char, s, l, src_ui);
+ bt_bitfield_read(target.c, unsigned short, s, l, &readval_ui);
+ if (check_result(src_ui, readval_ui, target.c, unsigned short,
+ s, l, UNSIGNED_INT_READ_TEST_DESC_FMT_STR)) {
+ return;
+ }
+
+ init_byte_array(target.c, TEST_LEN, 0xFF);
+ bt_bitfield_write(target.i, unsigned char, s, l, src_ui);
+ bt_bitfield_read(target.c, unsigned int, s, l, &readval_ui);
+ if (check_result(src_ui, readval_ui, target.c, unsigned int,
+ s, l, UNSIGNED_INT_READ_TEST_DESC_FMT_STR)) {
+ return;
+ }
+
+ init_byte_array(target.c, TEST_LEN, 0xFF);
+ bt_bitfield_write(target.l, unsigned char, s, l, src_ui);
+ bt_bitfield_read(target.c, unsigned long, s, l, &readval_ui);
+ if (check_result(src_ui, readval_ui, target.c, unsigned long,
+ s, l, UNSIGNED_INT_READ_TEST_DESC_FMT_STR)) {
+ return;
+ }
+
+ init_byte_array(target.c, TEST_LEN, 0xFF);
+ bt_bitfield_write(target.ll, unsigned char, s, l, src_ui);
+ bt_bitfield_read(target.c, unsigned long long, s, l, &readval_ui);
+ if (check_result(src_ui, readval_ui, target.c, unsigned long long,
+ s, l, UNSIGNED_INT_READ_TEST_DESC_FMT_STR)) {
+ return;
+ }
+ }
+ }
+ pass(UNSIGNED_INT_READ_TEST_DESC_FMT_STR, src_ui);
+
+ src_ull = srcrand_ull;
+ nrbits_ull = fls_u64(src_ull);
+
+ /* Read to unsigned long long readval output. */
+ for (s = 0; s < CHAR_BIT * TEST_LEN; s++) {
+ for (l = nrbits_ull; l < (CHAR_BIT * TEST_LEN) - s; l++) {
+ init_byte_array(target.c, TEST_LEN, 0xFF);
+ bt_bitfield_write(target.c, unsigned char, s, l, src_ull);
+ bt_bitfield_read(target.c, unsigned char, s, l, &readval_ull);
+ if (check_result(src_ull, readval_ull, target.c, unsigned char,
+ s, l, UNSIGNED_LONG_LONG_READ_TEST_DESC_FMT_STR)) {
+ return;
+ }
+
+ init_byte_array(target.c, TEST_LEN, 0xFF);
+ bt_bitfield_write(target.s, unsigned char, s, l, src_ull);
+ bt_bitfield_read(target.c, unsigned short, s, l, &readval_ull);
+ if (check_result(src_ull, readval_ull, target.c, unsigned short,
+ s, l, UNSIGNED_LONG_LONG_READ_TEST_DESC_FMT_STR)) {
+ return;
+ }
+
+ init_byte_array(target.c, TEST_LEN, 0xFF);
+ bt_bitfield_write(target.i, unsigned char, s, l, src_ull);
+ bt_bitfield_read(target.c, unsigned int, s, l, &readval_ull);
+ if (check_result(src_ull, readval_ull, target.c, unsigned int,
+ s, l, UNSIGNED_LONG_LONG_READ_TEST_DESC_FMT_STR)) {
+ return;
+ }
+
+ init_byte_array(target.c, TEST_LEN, 0xFF);
+ bt_bitfield_write(target.l, unsigned char, s, l, src_ull);
+ bt_bitfield_read(target.c, unsigned long, s, l, &readval_ull);
+ if (check_result(src_ull, readval_ull, target.c, unsigned long,
+ s, l, UNSIGNED_LONG_LONG_READ_TEST_DESC_FMT_STR)) {
+ return;
+ }
+
+ init_byte_array(target.c, TEST_LEN, 0xFF);
+ bt_bitfield_write(target.ll, unsigned char, s, l, src_ull);
+ bt_bitfield_read(target.c, unsigned long long, s, l, &readval_ull);
+ if (check_result(src_ull, readval_ull, target.c, unsigned long long,
+ s, l, UNSIGNED_LONG_LONG_READ_TEST_DESC_FMT_STR)) {
+ return;
+ }
+ }
+ }
+ pass(UNSIGNED_LONG_LONG_READ_TEST_DESC_FMT_STR, src_ull);
+}
+
+void run_test_unsigned(void)
+{
+ run_test_unsigned_write();
+ run_test_unsigned_read();
+}
+
+void run_test_signed_write(void)
+{
+ int src_i, nrbits_i, nrbits_ll;
union {
signed char c[TEST_LEN];
short s[TEST_LEN/sizeof(short)];
@@ -206,88 +442,272 @@ void run_test_signed(void)
long l[TEST_LEN/sizeof(long)];
long long ll[TEST_LEN/sizeof(long long)];
} target;
- long long readval;
+ long long src_ll, readval;
unsigned int s, l;
- src = srcrand;
- if (src & 0x80000000U)
- nrbits = fls(~src) + 1; /* Find least significant bit conveying sign */
+ src_i = srcrand_ui;
+ if (src_i & 0x80000000U)
+ nrbits_i = fls_u32(~src_i) + 1; /* Find least significant bit conveying sign */
+ else
+ nrbits_i = fls_u32(src_i) + 1; /* Keep sign at 0 */
+
+ /* Write from signed integer src input. */
+ for (s = 0; s < CHAR_BIT * TEST_LEN; s++) {
+ for (l = nrbits_i; l < (CHAR_BIT * TEST_LEN) - s; l++) {
+ init_byte_array(target.c, TEST_LEN, 0x0);
+ bt_bitfield_write(target.c, signed char, s, l, src_i);
+ bt_bitfield_read(target.c, signed char, s, l, &readval);
+ if (check_result(src_i, readval, target.c, signed char,
+ s, l, SIGNED_INT_WRITE_TEST_DESC_FMT_STR)) {
+ return;
+ }
+
+ init_byte_array(target.c, TEST_LEN, 0x0);
+ bt_bitfield_write(target.s, short, s, l, src_i);
+ bt_bitfield_read(target.c, signed char, s, l, &readval);
+ if (check_result(src_i, readval, target.c, short,
+ s, l, SIGNED_INT_WRITE_TEST_DESC_FMT_STR)) {
+ return;
+ }
+
+ init_byte_array(target.c, TEST_LEN, 0x0);
+ bt_bitfield_write(target.i, int, s, l, src_i);
+ bt_bitfield_read(target.c, signed char, s, l, &readval);
+ if (check_result(src_i, readval, target.c, int,
+ s, l, SIGNED_INT_WRITE_TEST_DESC_FMT_STR)) {
+ return;
+ }
+
+ init_byte_array(target.c, TEST_LEN, 0x0);
+ bt_bitfield_write(target.l, long, s, l, src_i);
+ bt_bitfield_read(target.c, signed char, s, l, &readval);
+ if (check_result(src_i, readval, target.c, long,
+ s, l, SIGNED_INT_WRITE_TEST_DESC_FMT_STR)) {
+ return;
+ }
+
+ init_byte_array(target.c, TEST_LEN, 0x0);
+ bt_bitfield_write(target.ll, long long, s, l, src_i);
+ bt_bitfield_read(target.c, signed char, s, l, &readval);
+ if (check_result(src_i, readval, target.c, long long,
+ s, l, SIGNED_INT_WRITE_TEST_DESC_FMT_STR)) {
+ return;
+ }
+ }
+ }
+ pass(SIGNED_INT_WRITE_TEST_DESC_FMT_STR, src_i);
+
+ src_ll = srcrand_ull;
+ if (src_ll & 0x8000000000000000ULL)
+ nrbits_ll = fls_u64(~src_ll) + 1; /* Find least significant bit conveying sign */
else
- nrbits = fls(src) + 1; /* Keep sign at 0 */
+ nrbits_ll = fls_u64(src_ll) + 1; /* Keep sign at 0 */
+ /* Write from signed long long src input. */
for (s = 0; s < CHAR_BIT * TEST_LEN; s++) {
- for (l = nrbits; l < (CHAR_BIT * TEST_LEN) - s; l++) {
+ for (l = nrbits_ll; l < (CHAR_BIT * TEST_LEN) - s; l++) {
init_byte_array(target.c, TEST_LEN, 0x0);
- bt_bitfield_write(target.c, signed char, s, l, src);
+ bt_bitfield_write(target.c, signed char, s, l, src_ll);
bt_bitfield_read(target.c, signed char, s, l, &readval);
- if (check_result(src, readval, target.c, signed char,
- s, l, SIGNED_TEST_DESC_FMT_STR)) {
+ if (check_result(src_ll, readval, target.c, signed char,
+ s, l, SIGNED_LONG_LONG_WRITE_TEST_DESC_FMT_STR)) {
return;
}
init_byte_array(target.c, TEST_LEN, 0x0);
- bt_bitfield_write(target.s, short, s, l, src);
+ bt_bitfield_write(target.s, short, s, l, src_ll);
bt_bitfield_read(target.c, signed char, s, l, &readval);
- if (check_result(src, readval, target.c, short,
- s, l, SIGNED_TEST_DESC_FMT_STR)) {
+ if (check_result(src_ll, readval, target.c, short,
+ s, l, SIGNED_LONG_LONG_WRITE_TEST_DESC_FMT_STR)) {
return;
}
init_byte_array(target.c, TEST_LEN, 0x0);
- bt_bitfield_write(target.i, int, s, l, src);
+ bt_bitfield_write(target.i, int, s, l, src_ll);
bt_bitfield_read(target.c, signed char, s, l, &readval);
- if (check_result(src, readval, target.c, int,
- s, l, SIGNED_TEST_DESC_FMT_STR)) {
+ if (check_result(src_ll, readval, target.c, int,
+ s, l, SIGNED_LONG_LONG_WRITE_TEST_DESC_FMT_STR)) {
return;
}
init_byte_array(target.c, TEST_LEN, 0x0);
- bt_bitfield_write(target.l, long, s, l, src);
+ bt_bitfield_write(target.l, long, s, l, src_ll);
bt_bitfield_read(target.c, signed char, s, l, &readval);
- if (check_result(src, readval, target.c, long,
- s, l, SIGNED_TEST_DESC_FMT_STR)) {
+ if (check_result(src_ll, readval, target.c, long,
+ s, l, SIGNED_LONG_LONG_WRITE_TEST_DESC_FMT_STR)) {
return;
}
init_byte_array(target.c, TEST_LEN, 0x0);
- bt_bitfield_write(target.ll, long long, s, l, src);
+ bt_bitfield_write(target.ll, long long, s, l, src_ll);
bt_bitfield_read(target.c, signed char, s, l, &readval);
- if (check_result(src, readval, target.c, long long,
- s, l, SIGNED_TEST_DESC_FMT_STR)) {
+ if (check_result(src_ll, readval, target.c, long long,
+ s, l, SIGNED_LONG_LONG_WRITE_TEST_DESC_FMT_STR)) {
return;
}
}
}
+ pass(SIGNED_LONG_LONG_WRITE_TEST_DESC_FMT_STR, src_ll);
+}
- pass(SIGNED_TEST_DESC_FMT_STR, src);
+void run_test_signed_read(void)
+{
+ int src_i, nrbits_i, nrbits_ll, readval_i;
+ union {
+ unsigned char c[TEST_LEN];
+ unsigned short s[TEST_LEN/sizeof(unsigned short)];
+ unsigned int i[TEST_LEN/sizeof(unsigned int)];
+ unsigned long l[TEST_LEN/sizeof(unsigned long)];
+ unsigned long long ll[TEST_LEN/sizeof(unsigned long long)];
+ } target;
+ long long src_ll, readval_ll;
+ unsigned int s, l;
+
+ src_i = srcrand_ui;
+ if (src_i & 0x80000000U)
+ nrbits_i = fls_u32(~src_i) + 1; /* Find least significant bit conveying sign */
+ else
+ nrbits_i = fls_u32(src_i) + 1; /* Keep sign at 0 */
+
+ /* Read to signed integer readval output. */
+ for (s = 0; s < CHAR_BIT * TEST_LEN; s++) {
+ for (l = nrbits_i; l < (CHAR_BIT * TEST_LEN) - s; l++) {
+ init_byte_array(target.c, TEST_LEN, 0xFF);
+ bt_bitfield_write(target.c, signed char, s, l, src_i);
+ bt_bitfield_read(target.c, signed char, s, l, &readval_i);
+ if (check_result(src_i, readval_i, target.c, signed char,
+ s, l, SIGNED_INT_READ_TEST_DESC_FMT_STR)) {
+ return;
+ }
+
+ init_byte_array(target.c, TEST_LEN, 0xFF);
+ bt_bitfield_write(target.s, signed char, s, l, src_i);
+ bt_bitfield_read(target.c, short, s, l, &readval_i);
+ if (check_result(src_i, readval_i, target.c, short,
+ s, l, SIGNED_INT_READ_TEST_DESC_FMT_STR)) {
+ return;
+ }
+
+ init_byte_array(target.c, TEST_LEN, 0xFF);
+ bt_bitfield_write(target.i, signed char, s, l, src_i);
+ bt_bitfield_read(target.c, int, s, l, &readval_i);
+ if (check_result(src_i, readval_i, target.c, int,
+ s, l, SIGNED_INT_READ_TEST_DESC_FMT_STR)) {
+ return;
+ }
+
+ init_byte_array(target.c, TEST_LEN, 0xFF);
+ bt_bitfield_write(target.l, signed char, s, l, src_i);
+ bt_bitfield_read(target.c, long, s, l, &readval_i);
+ if (check_result(src_i, readval_i, target.c, long,
+ s, l, SIGNED_INT_READ_TEST_DESC_FMT_STR)) {
+ return;
+ }
+
+ init_byte_array(target.c, TEST_LEN, 0xFF);
+ bt_bitfield_write(target.ll, signed char, s, l, src_i);
+ bt_bitfield_read(target.c, long long, s, l, &readval_i);
+ if (check_result(src_i, readval_i, target.c, long long,
+ s, l, SIGNED_INT_READ_TEST_DESC_FMT_STR)) {
+ return;
+ }
+ }
+ }
+ pass(SIGNED_INT_READ_TEST_DESC_FMT_STR, src_i);
+
+ src_ll = srcrand_ull;
+ if (src_ll & 0x8000000000000000ULL)
+ nrbits_ll = fls_u64(~src_ll) + 1; /* Find least significant bit conveying sign */
+ else
+ nrbits_ll = fls_u64(src_ll) + 1; /* Keep sign at 0 */
+
+ /* Read to unsigned long long readval output. */
+ for (s = 0; s < CHAR_BIT * TEST_LEN; s++) {
+ for (l = nrbits_ll; l < (CHAR_BIT * TEST_LEN) - s; l++) {
+ init_byte_array(target.c, TEST_LEN, 0xFF);
+ bt_bitfield_write(target.c, signed char, s, l, src_ll);
+ bt_bitfield_read(target.c, signed char, s, l, &readval_ll);
+ if (check_result(src_ll, readval_ll, target.c, signed char,
+ s, l, SIGNED_LONG_LONG_READ_TEST_DESC_FMT_STR)) {
+ return;
+ }
+
+ init_byte_array(target.c, TEST_LEN, 0xFF);
+ bt_bitfield_write(target.s, signed char, s, l, src_ll);
+ bt_bitfield_read(target.c, short, s, l, &readval_ll);
+ if (check_result(src_ll, readval_ll, target.c, short,
+ s, l, SIGNED_LONG_LONG_READ_TEST_DESC_FMT_STR)) {
+ return;
+ }
+
+ init_byte_array(target.c, TEST_LEN, 0xFF);
+ bt_bitfield_write(target.i, signed char, s, l, src_ll);
+ bt_bitfield_read(target.c, int, s, l, &readval_ll);
+ if (check_result(src_ll, readval_ll, target.c, int,
+ s, l, SIGNED_LONG_LONG_READ_TEST_DESC_FMT_STR)) {
+ return;
+ }
+
+ init_byte_array(target.c, TEST_LEN, 0xFF);
+ bt_bitfield_write(target.l, signed char, s, l, src_ll);
+ bt_bitfield_read(target.c, long, s, l, &readval_ll);
+ if (check_result(src_ll, readval_ll, target.c, long,
+ s, l, SIGNED_LONG_LONG_READ_TEST_DESC_FMT_STR)) {
+ return;
+ }
+
+ init_byte_array(target.c, TEST_LEN, 0xFF);
+ bt_bitfield_write(target.ll, signed char, s, l, src_ll);
+ bt_bitfield_read(target.c, long long, s, l, &readval_ll);
+ if (check_result(src_ll, readval_ll, target.c, long long,
+ s, l, SIGNED_LONG_LONG_READ_TEST_DESC_FMT_STR)) {
+ return;
+ }
+ }
+ }
+ pass(SIGNED_LONG_LONG_READ_TEST_DESC_FMT_STR, src_ll);
+}
+
+void run_test_signed(void)
+{
+ run_test_signed_write();
+ run_test_signed_read();
}
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();
- srcrand = 0;
+ srcrand_ui = 0;
+ srcrand_ull = 0;
run_test_signed();
- srcrand = 1;
+ srcrand_ui = 1;
+ srcrand_ull = 1;
run_test_unsigned();
- srcrand = ~0U;
+ srcrand_ui = ~0U;
+ srcrand_ull = ~0ULL;
run_test_unsigned();
- srcrand = -1;
+ srcrand_ui = -1U;
+ srcrand_ull = -1ULL;
run_test_signed();
- srcrand = (int)0x80000000U;
+ srcrand_ui = 0x80000000U;
+ srcrand_ull = 0x8000000000000000ULL;
run_test_signed();
for (i = 0; i < NR_TESTS; i++) {
- srcrand = rand();
+ srcrand_ui = rand();
+ srcrand_ull = ((unsigned long long) (unsigned int) rand() << 32) |
+ (unsigned long long) (unsigned int) rand();
run_test_unsigned();
run_test_signed();
}
--
2.11.0
^ permalink raw reply [flat|nested] 5+ messages in thread
* [lttng-dev] [PATCH babeltrace 4/4] Cleanup: test_bitfield: pass source integers as parameters
2019-05-15 21:46 [lttng-dev] [PATCH v7 babeltrace 1/4] Fix: bitfield: shift undefined/implementation defined behaviors Mathieu Desnoyers
2019-05-15 21:46 ` [lttng-dev] [PATCH babeltrace 2/4] Silence compiler "always false comparison" warning Mathieu Desnoyers
2019-05-15 21:46 ` [lttng-dev] [PATCH babeltrace 3/4] Extend test_bitfield coverage Mathieu Desnoyers
@ 2019-05-15 21:46 ` Mathieu Desnoyers
2 siblings, 0 replies; 5+ messages in thread
From: Mathieu Desnoyers @ 2019-05-15 21:46 UTC (permalink / raw)
It is a good practice to pass arguments as parameters rather
than through global variables.
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers at efficios.com>
Change-Id: I0d7cc94b1d492ca53ad29b1d39831ee6288b2672
---
tests/lib/test_bitfield.c | 85 +++++++++++++++++------------------------------
1 file changed, 30 insertions(+), 55 deletions(-)
diff --git a/tests/lib/test_bitfield.c b/tests/lib/test_bitfield.c
index 59bf89fd..63168fcd 100644
--- a/tests/lib/test_bitfield.c
+++ b/tests/lib/test_bitfield.c
@@ -51,9 +51,6 @@ void fct(void)
#define DIAG_FMT_STR "Failed reading value written \"%s\"-wise, with start=%i" \
" and length=%i. Read %llX"
-unsigned int srcrand_ui;
-unsigned long long srcrand_ull;
-
/*
* fls: returns the position of the most significant bit.
* Returns 0 if no bit is set, else returns the position of the most
@@ -202,9 +199,9 @@ do { \
(val) != (ref); \
})
-void run_test_unsigned_write(void)
+void run_test_unsigned_write(unsigned int src_ui, unsigned long long src_ull)
{
- unsigned int src_ui, nrbits_ui, nrbits_ull;
+ unsigned int nrbits_ui, nrbits_ull;
union {
unsigned char c[TEST_LEN];
unsigned short s[TEST_LEN/sizeof(unsigned short)];
@@ -212,10 +209,9 @@ void run_test_unsigned_write(void)
unsigned long l[TEST_LEN/sizeof(unsigned long)];
unsigned long long ll[TEST_LEN/sizeof(unsigned long long)];
} target;
- unsigned long long src_ull, readval;
+ unsigned long long readval;
unsigned int s, l;
- src_ui = srcrand_ui;
nrbits_ui = fls_u32(src_ui);
/* Write from unsigned integer src input. */
@@ -264,7 +260,6 @@ void run_test_unsigned_write(void)
}
pass(UNSIGNED_INT_WRITE_TEST_DESC_FMT_STR, src_ui);
- src_ull = srcrand_ull;
nrbits_ull = fls_u64(src_ull);
/* Write from unsigned long long src input. */
@@ -314,9 +309,9 @@ void run_test_unsigned_write(void)
pass(UNSIGNED_LONG_LONG_WRITE_TEST_DESC_FMT_STR, src_ull);
}
-void run_test_unsigned_read(void)
+void run_test_unsigned_read(unsigned int src_ui, unsigned long long src_ull)
{
- unsigned int src_ui, nrbits_ui, nrbits_ull, readval_ui;
+ unsigned int nrbits_ui, nrbits_ull, readval_ui;
union {
unsigned char c[TEST_LEN];
unsigned short s[TEST_LEN/sizeof(unsigned short)];
@@ -324,10 +319,9 @@ void run_test_unsigned_read(void)
unsigned long l[TEST_LEN/sizeof(unsigned long)];
unsigned long long ll[TEST_LEN/sizeof(unsigned long long)];
} target;
- unsigned long long src_ull, readval_ull;
+ unsigned long long readval_ull;
unsigned int s, l;
- src_ui = srcrand_ui;
nrbits_ui = fls_u32(src_ui);
/* Read to unsigned integer readval output. */
@@ -376,7 +370,6 @@ void run_test_unsigned_read(void)
}
pass(UNSIGNED_INT_READ_TEST_DESC_FMT_STR, src_ui);
- src_ull = srcrand_ull;
nrbits_ull = fls_u64(src_ull);
/* Read to unsigned long long readval output. */
@@ -426,15 +419,15 @@ void run_test_unsigned_read(void)
pass(UNSIGNED_LONG_LONG_READ_TEST_DESC_FMT_STR, src_ull);
}
-void run_test_unsigned(void)
+void run_test_unsigned(unsigned int src_ui, unsigned long long src_ull)
{
- run_test_unsigned_write();
- run_test_unsigned_read();
+ run_test_unsigned_write(src_ui, src_ull);
+ run_test_unsigned_read(src_ui, src_ull);
}
-void run_test_signed_write(void)
+void run_test_signed_write(int src_i, long long src_ll)
{
- int src_i, nrbits_i, nrbits_ll;
+ int nrbits_i, nrbits_ll;
union {
signed char c[TEST_LEN];
short s[TEST_LEN/sizeof(short)];
@@ -442,10 +435,9 @@ void run_test_signed_write(void)
long l[TEST_LEN/sizeof(long)];
long long ll[TEST_LEN/sizeof(long long)];
} target;
- long long src_ll, readval;
+ long long readval;
unsigned int s, l;
- src_i = srcrand_ui;
if (src_i & 0x80000000U)
nrbits_i = fls_u32(~src_i) + 1; /* Find least significant bit conveying sign */
else
@@ -497,7 +489,6 @@ void run_test_signed_write(void)
}
pass(SIGNED_INT_WRITE_TEST_DESC_FMT_STR, src_i);
- src_ll = srcrand_ull;
if (src_ll & 0x8000000000000000ULL)
nrbits_ll = fls_u64(~src_ll) + 1; /* Find least significant bit conveying sign */
else
@@ -550,9 +541,9 @@ void run_test_signed_write(void)
pass(SIGNED_LONG_LONG_WRITE_TEST_DESC_FMT_STR, src_ll);
}
-void run_test_signed_read(void)
+void run_test_signed_read(int src_i, long long src_ll)
{
- int src_i, nrbits_i, nrbits_ll, readval_i;
+ int nrbits_i, nrbits_ll, readval_i;
union {
unsigned char c[TEST_LEN];
unsigned short s[TEST_LEN/sizeof(unsigned short)];
@@ -560,10 +551,9 @@ void run_test_signed_read(void)
unsigned long l[TEST_LEN/sizeof(unsigned long)];
unsigned long long ll[TEST_LEN/sizeof(unsigned long long)];
} target;
- long long src_ll, readval_ll;
+ long long readval_ll;
unsigned int s, l;
- src_i = srcrand_ui;
if (src_i & 0x80000000U)
nrbits_i = fls_u32(~src_i) + 1; /* Find least significant bit conveying sign */
else
@@ -615,7 +605,6 @@ void run_test_signed_read(void)
}
pass(SIGNED_INT_READ_TEST_DESC_FMT_STR, src_i);
- src_ll = srcrand_ull;
if (src_ll & 0x8000000000000000ULL)
nrbits_ll = fls_u64(~src_ll) + 1; /* Find least significant bit conveying sign */
else
@@ -668,48 +657,34 @@ void run_test_signed_read(void)
pass(SIGNED_LONG_LONG_READ_TEST_DESC_FMT_STR, src_ll);
}
-void run_test_signed(void)
+void run_test_signed(int src_i, long long src_ll)
{
- run_test_signed_write();
- run_test_signed_read();
+ run_test_signed_write(src_i, src_ll);
+ run_test_signed_read(src_i, src_ll);
}
void run_test(void)
{
int i;
+
plan_tests(NR_TESTS * 8 + 24);
srand(time(NULL));
- srcrand_ui = 0;
- srcrand_ull = 0;
- run_test_unsigned();
- srcrand_ui = 0;
- srcrand_ull = 0;
- run_test_signed();
-
- srcrand_ui = 1;
- srcrand_ull = 1;
- run_test_unsigned();
-
- srcrand_ui = ~0U;
- srcrand_ull = ~0ULL;
- run_test_unsigned();
-
- srcrand_ui = -1U;
- srcrand_ull = -1ULL;
- run_test_signed();
-
- srcrand_ui = 0x80000000U;
- srcrand_ull = 0x8000000000000000ULL;
- run_test_signed();
+ run_test_unsigned(0, 0);
+ run_test_signed(0, 0);
+ run_test_unsigned(1, 1);
+ run_test_unsigned(~0U, ~0ULL);
+ run_test_signed(-1U, -1ULL);
+ run_test_signed(0x80000000U, 0x8000000000000000ULL);
for (i = 0; i < NR_TESTS; i++) {
- srcrand_ui = rand();
- srcrand_ull = ((unsigned long long) (unsigned int) rand() << 32) |
+ unsigned int src_ui = rand();
+ unsigned long long src_ull = ((unsigned long long) (unsigned int) rand() << 32) |
(unsigned long long) (unsigned int) rand();
- run_test_unsigned();
- run_test_signed();
+
+ run_test_unsigned(src_ui, src_ull);
+ run_test_signed((int) src_ui, (long long) src_ull);
}
}
--
2.11.0
^ permalink raw reply [flat|nested] 5+ messages in thread
* [lttng-dev] [PATCH babeltrace 4/4] Cleanup: test_bitfield: pass source integers as parameters
2019-05-14 20:30 [lttng-dev] [PATCH babeltrace 1/4] Fix: bitfield: shift undefined/implementation defined behaviors (v6) Mathieu Desnoyers
@ 2019-05-14 20:30 ` Mathieu Desnoyers
0 siblings, 0 replies; 5+ messages in thread
From: Mathieu Desnoyers @ 2019-05-14 20:30 UTC (permalink / raw)
It is a good practice to pass arguments as parameters rather
than through global variables.
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers at efficios.com>
Change-Id: I0d7cc94b1d492ca53ad29b1d39831ee6288b2672
---
tests/lib/test_bitfield.c | 85 +++++++++++++++++------------------------------
1 file changed, 30 insertions(+), 55 deletions(-)
diff --git a/tests/lib/test_bitfield.c b/tests/lib/test_bitfield.c
index 59bf89fd..63168fcd 100644
--- a/tests/lib/test_bitfield.c
+++ b/tests/lib/test_bitfield.c
@@ -51,9 +51,6 @@ void fct(void)
#define DIAG_FMT_STR "Failed reading value written \"%s\"-wise, with start=%i" \
" and length=%i. Read %llX"
-unsigned int srcrand_ui;
-unsigned long long srcrand_ull;
-
/*
* fls: returns the position of the most significant bit.
* Returns 0 if no bit is set, else returns the position of the most
@@ -202,9 +199,9 @@ do { \
(val) != (ref); \
})
-void run_test_unsigned_write(void)
+void run_test_unsigned_write(unsigned int src_ui, unsigned long long src_ull)
{
- unsigned int src_ui, nrbits_ui, nrbits_ull;
+ unsigned int nrbits_ui, nrbits_ull;
union {
unsigned char c[TEST_LEN];
unsigned short s[TEST_LEN/sizeof(unsigned short)];
@@ -212,10 +209,9 @@ void run_test_unsigned_write(void)
unsigned long l[TEST_LEN/sizeof(unsigned long)];
unsigned long long ll[TEST_LEN/sizeof(unsigned long long)];
} target;
- unsigned long long src_ull, readval;
+ unsigned long long readval;
unsigned int s, l;
- src_ui = srcrand_ui;
nrbits_ui = fls_u32(src_ui);
/* Write from unsigned integer src input. */
@@ -264,7 +260,6 @@ void run_test_unsigned_write(void)
}
pass(UNSIGNED_INT_WRITE_TEST_DESC_FMT_STR, src_ui);
- src_ull = srcrand_ull;
nrbits_ull = fls_u64(src_ull);
/* Write from unsigned long long src input. */
@@ -314,9 +309,9 @@ void run_test_unsigned_write(void)
pass(UNSIGNED_LONG_LONG_WRITE_TEST_DESC_FMT_STR, src_ull);
}
-void run_test_unsigned_read(void)
+void run_test_unsigned_read(unsigned int src_ui, unsigned long long src_ull)
{
- unsigned int src_ui, nrbits_ui, nrbits_ull, readval_ui;
+ unsigned int nrbits_ui, nrbits_ull, readval_ui;
union {
unsigned char c[TEST_LEN];
unsigned short s[TEST_LEN/sizeof(unsigned short)];
@@ -324,10 +319,9 @@ void run_test_unsigned_read(void)
unsigned long l[TEST_LEN/sizeof(unsigned long)];
unsigned long long ll[TEST_LEN/sizeof(unsigned long long)];
} target;
- unsigned long long src_ull, readval_ull;
+ unsigned long long readval_ull;
unsigned int s, l;
- src_ui = srcrand_ui;
nrbits_ui = fls_u32(src_ui);
/* Read to unsigned integer readval output. */
@@ -376,7 +370,6 @@ void run_test_unsigned_read(void)
}
pass(UNSIGNED_INT_READ_TEST_DESC_FMT_STR, src_ui);
- src_ull = srcrand_ull;
nrbits_ull = fls_u64(src_ull);
/* Read to unsigned long long readval output. */
@@ -426,15 +419,15 @@ void run_test_unsigned_read(void)
pass(UNSIGNED_LONG_LONG_READ_TEST_DESC_FMT_STR, src_ull);
}
-void run_test_unsigned(void)
+void run_test_unsigned(unsigned int src_ui, unsigned long long src_ull)
{
- run_test_unsigned_write();
- run_test_unsigned_read();
+ run_test_unsigned_write(src_ui, src_ull);
+ run_test_unsigned_read(src_ui, src_ull);
}
-void run_test_signed_write(void)
+void run_test_signed_write(int src_i, long long src_ll)
{
- int src_i, nrbits_i, nrbits_ll;
+ int nrbits_i, nrbits_ll;
union {
signed char c[TEST_LEN];
short s[TEST_LEN/sizeof(short)];
@@ -442,10 +435,9 @@ void run_test_signed_write(void)
long l[TEST_LEN/sizeof(long)];
long long ll[TEST_LEN/sizeof(long long)];
} target;
- long long src_ll, readval;
+ long long readval;
unsigned int s, l;
- src_i = srcrand_ui;
if (src_i & 0x80000000U)
nrbits_i = fls_u32(~src_i) + 1; /* Find least significant bit conveying sign */
else
@@ -497,7 +489,6 @@ void run_test_signed_write(void)
}
pass(SIGNED_INT_WRITE_TEST_DESC_FMT_STR, src_i);
- src_ll = srcrand_ull;
if (src_ll & 0x8000000000000000ULL)
nrbits_ll = fls_u64(~src_ll) + 1; /* Find least significant bit conveying sign */
else
@@ -550,9 +541,9 @@ void run_test_signed_write(void)
pass(SIGNED_LONG_LONG_WRITE_TEST_DESC_FMT_STR, src_ll);
}
-void run_test_signed_read(void)
+void run_test_signed_read(int src_i, long long src_ll)
{
- int src_i, nrbits_i, nrbits_ll, readval_i;
+ int nrbits_i, nrbits_ll, readval_i;
union {
unsigned char c[TEST_LEN];
unsigned short s[TEST_LEN/sizeof(unsigned short)];
@@ -560,10 +551,9 @@ void run_test_signed_read(void)
unsigned long l[TEST_LEN/sizeof(unsigned long)];
unsigned long long ll[TEST_LEN/sizeof(unsigned long long)];
} target;
- long long src_ll, readval_ll;
+ long long readval_ll;
unsigned int s, l;
- src_i = srcrand_ui;
if (src_i & 0x80000000U)
nrbits_i = fls_u32(~src_i) + 1; /* Find least significant bit conveying sign */
else
@@ -615,7 +605,6 @@ void run_test_signed_read(void)
}
pass(SIGNED_INT_READ_TEST_DESC_FMT_STR, src_i);
- src_ll = srcrand_ull;
if (src_ll & 0x8000000000000000ULL)
nrbits_ll = fls_u64(~src_ll) + 1; /* Find least significant bit conveying sign */
else
@@ -668,48 +657,34 @@ void run_test_signed_read(void)
pass(SIGNED_LONG_LONG_READ_TEST_DESC_FMT_STR, src_ll);
}
-void run_test_signed(void)
+void run_test_signed(int src_i, long long src_ll)
{
- run_test_signed_write();
- run_test_signed_read();
+ run_test_signed_write(src_i, src_ll);
+ run_test_signed_read(src_i, src_ll);
}
void run_test(void)
{
int i;
+
plan_tests(NR_TESTS * 8 + 24);
srand(time(NULL));
- srcrand_ui = 0;
- srcrand_ull = 0;
- run_test_unsigned();
- srcrand_ui = 0;
- srcrand_ull = 0;
- run_test_signed();
-
- srcrand_ui = 1;
- srcrand_ull = 1;
- run_test_unsigned();
-
- srcrand_ui = ~0U;
- srcrand_ull = ~0ULL;
- run_test_unsigned();
-
- srcrand_ui = -1U;
- srcrand_ull = -1ULL;
- run_test_signed();
-
- srcrand_ui = 0x80000000U;
- srcrand_ull = 0x8000000000000000ULL;
- run_test_signed();
+ run_test_unsigned(0, 0);
+ run_test_signed(0, 0);
+ run_test_unsigned(1, 1);
+ run_test_unsigned(~0U, ~0ULL);
+ run_test_signed(-1U, -1ULL);
+ run_test_signed(0x80000000U, 0x8000000000000000ULL);
for (i = 0; i < NR_TESTS; i++) {
- srcrand_ui = rand();
- srcrand_ull = ((unsigned long long) (unsigned int) rand() << 32) |
+ unsigned int src_ui = rand();
+ unsigned long long src_ull = ((unsigned long long) (unsigned int) rand() << 32) |
(unsigned long long) (unsigned int) rand();
- run_test_unsigned();
- run_test_signed();
+
+ run_test_unsigned(src_ui, src_ull);
+ run_test_signed((int) src_ui, (long long) src_ull);
}
}
--
2.11.0
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-05-15 21:46 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-15 21:46 [lttng-dev] [PATCH v7 babeltrace 1/4] Fix: bitfield: shift undefined/implementation defined behaviors Mathieu Desnoyers
2019-05-15 21:46 ` [lttng-dev] [PATCH babeltrace 2/4] Silence compiler "always false comparison" warning Mathieu Desnoyers
2019-05-15 21:46 ` [lttng-dev] [PATCH babeltrace 3/4] Extend test_bitfield coverage Mathieu Desnoyers
2019-05-15 21:46 ` [lttng-dev] [PATCH babeltrace 4/4] Cleanup: test_bitfield: pass source integers as parameters Mathieu Desnoyers
-- strict thread matches above, loose matches on Subject: below --
2019-05-14 20:30 [lttng-dev] [PATCH babeltrace 1/4] Fix: bitfield: shift undefined/implementation defined behaviors (v6) Mathieu Desnoyers
2019-05-14 20:30 ` [lttng-dev] [PATCH babeltrace 4/4] Cleanup: test_bitfield: pass source integers as parameters Mathieu Desnoyers
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox