Commit 4f79b91d authored by Petr Špaček's avatar Petr Špaček

rplan: fix kr_qflags_*() to work with more than 32 flags

Originally division around sizeof(uint32_t) caused silent truncation
for struct kr_qflags with sizes not multiple of 4 bytes.

Attempts to optimize using uint32_t blocks could lead to read/write
beyond end of uint32_t so I'm not willing to risk it.

Also, the code was refactored to avoid duplication between _set and _clear.
Quick look into assembly produced by gcc 7.2.1 with -O2 on x86_64 confirms that
all auxiliary functions got inlined so there are not extra function calls.

Unit tests are attached. These fail on the previous version of _set() and
_clear() and work now.
parent c905ff8e
......@@ -27,37 +27,58 @@
#define QUERY_PROVIDES(q, name, cls, type) \
((q)->sclass == (cls) && (q)->stype == type && knot_dname_is_equal((q)->sname, name))
void kr_qflags_set(struct kr_qflags *fl1, struct kr_qflags fl2)
inline static unsigned char chars_or(const unsigned char a, const unsigned char b)
{
return a | b;
}
/** Bits set to 1 in variable b will be set to zero in variable a. */
inline static unsigned char chars_mask(const unsigned char a, const unsigned char b)
{
return a & ~b;
}
/** Apply mod(a, b) to every byte a, b from fl1, fl2 and return result in fl1. */
inline static void kr_qflags_mod(struct kr_qflags *fl1, struct kr_qflags fl2,
unsigned char mod(const unsigned char a, const unsigned char b))
{
if (!fl1) abort();
typedef uint32_t ui;
union {
struct kr_qflags flags;
ui uints[sizeof(struct kr_qflags) / sizeof(ui)];
/* C99 section 6.5.3.4: sizeof(char) == 1 */
unsigned char chars[sizeof(struct kr_qflags)];
} tmp1, tmp2;
/* The compiler should be able to optimize all this into simple ORs. */
tmp1.flags = *fl1;
tmp2.flags = fl2;
for (size_t i = 0; i < sizeof(tmp1.uints) / sizeof(tmp1.uints[0]); ++i) {
tmp1.uints[i] |= tmp2.uints[i];
for (size_t i = 0; i < sizeof(struct kr_qflags); ++i) {
tmp1.chars[i] = mod(tmp1.chars[i], tmp2.chars[i]);
}
*fl1 = tmp1.flags;
}
/**
* Set bits from variable fl2 in variable fl1.
* Bits which are not set in fl2 are not modified in fl1.
*
* @param[in,out] fl1
* @param[in] fl2
*/
void kr_qflags_set(struct kr_qflags *fl1, struct kr_qflags fl2)
{
kr_qflags_mod(fl1, fl2, chars_or);
}
/**
* Clear bits from variable fl2 in variable fl1.
* Bits which are not set in fl2 are not modified in fl1.
*
* @param[in,out] fl1
* @param[in] fl2
*/
void kr_qflags_clear(struct kr_qflags *fl1, struct kr_qflags fl2)
{
if (!fl1) abort();
typedef uint32_t ui;
union {
struct kr_qflags flags;
ui uints[sizeof(struct kr_qflags) / sizeof(ui)];
} tmp1, tmp2;
/* The compiler should be able to optimize all this into simple ORs. */
tmp1.flags = *fl1;
tmp2.flags = fl2;
for (size_t i = 0; i < sizeof(tmp1.uints) / sizeof(tmp1.uints[0]); ++i) {
tmp1.uints[i] &= ~tmp2.uints[i];
}
*fl1 = tmp1.flags;
kr_qflags_mod(fl1, fl2, chars_mask);
}
static struct kr_query *query_create(knot_mm_t *pool, const knot_dname_t *name, uint32_t uid)
......
......@@ -56,11 +56,31 @@ static void test_rplan_push(void **state)
kr_rplan_deinit(&rplan);
}
/**
* Set and clear must not omit any bit, especially around byte boundaries.
*/
static void test_rplan_flags(void **state)
{
static struct kr_qflags f1, f2, ones, zeros; /* static => initialized to zeroes */
assert_true(memcmp(&f1, &f2, sizeof(f1)) == 0); /* sanity check */
memset(&ones, 0xff, sizeof(ones)); /* all ones */
/* test set */
kr_qflags_set(&f1, ones);
assert_true(memcmp(&f1, &ones, sizeof(f1)) == 0); /* 1 == 1 */
/* test clear */
memset(&f2, 0xff, sizeof(f2)); /* all ones */
kr_qflags_clear(&f2, ones);
assert_true(memcmp(&f2, &zeros, sizeof(f1)) == 0); /* 0 == 0 */
}
int main(void)
{
const UnitTest tests[] = {
unit_test(test_rplan_params),
unit_test(test_rplan_push)
unit_test(test_rplan_push),
unit_test(test_rplan_flags)
};
return run_tests(tests);
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment