Skip to content

equeue: avoid non-standard bit shifts #11782

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Nov 6, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 7 additions & 7 deletions events/source/equeue.c
Original file line number Diff line number Diff line change
Expand Up @@ -36,14 +36,14 @@ static inline int equeue_tickdiff(unsigned a, unsigned b)
static inline int equeue_clampdiff(unsigned a, unsigned b)
{
int diff = equeue_tickdiff(a, b);
return ~(diff >> (8 * sizeof(int) -1)) & diff;
return diff > 0 ? diff : 0;
}

// Increment the unique id in an event, hiding the event from cancel
static inline void equeue_incid(equeue_t *q, struct equeue_event *e)
{
e->id += 1;
if ((e->id << q->npw2) == 0) {
if (((unsigned)e->id << q->npw2) == 0) {
e->id = 1;
}
}
Expand Down Expand Up @@ -280,7 +280,7 @@ void equeue_enqueue(equeue_t *q, struct equeue_event *e, unsigned tick)
static int equeue_event_id(equeue_t *q, struct equeue_event *e)
{
// setup event and hash local id with buffer offset for unique id
return ((e->id << q->npw2) | ((unsigned char *)e - q->buffer));
return ((unsigned)e->id << q->npw2) | ((unsigned char *)e - q->buffer);
}

static struct equeue_event *equeue_unqueue_by_address(equeue_t *q, struct equeue_event *e)
Expand Down Expand Up @@ -319,10 +319,10 @@ static struct equeue_event *equeue_unqueue_by_id(equeue_t *q, int id)
{
// decode event from unique id and check that the local id matches
struct equeue_event *e = (struct equeue_event *)
&q->buffer[id & ((1 << q->npw2) - 1)];
&q->buffer[id & ((1u << q->npw2) - 1u)];

equeue_mutex_lock(&q->queuelock);
if (e->id != id >> q->npw2) {
if (e->id != (unsigned)id >> q->npw2) {
equeue_mutex_unlock(&q->queuelock);
return 0;
}
Expand Down Expand Up @@ -447,10 +447,10 @@ int equeue_timeleft(equeue_t *q, int id)

// decode event from unique id and check that the local id matches
struct equeue_event *e = (struct equeue_event *)
&q->buffer[id & ((1 << q->npw2) - 1)];
&q->buffer[id & ((1u << q->npw2) - 1u)];

equeue_mutex_lock(&q->queuelock);
if (e->id == id >> q->npw2) {
if (e->id == (unsigned)id >> q->npw2) {
ret = equeue_clampdiff(e->target, equeue_tick());
}
equeue_mutex_unlock(&q->queuelock);
Expand Down
42 changes: 29 additions & 13 deletions events/source/tests/tests.c
Original file line number Diff line number Diff line change
Expand Up @@ -297,7 +297,7 @@ void cancel_test(int N)
}

for (int i = N - 1; i >= 0; i--) {
equeue_cancel(&q, ids[i]);
test_assert(equeue_cancel(&q, ids[i]));
}

free(ids);
Expand All @@ -317,13 +317,13 @@ void cancel_inflight_test(void)
bool touched = false;

int id = equeue_call(&q, simple_func, &touched);
equeue_cancel(&q, id);
test_assert(equeue_cancel(&q, id));

equeue_dispatch(&q, 0);
test_assert(!touched);

id = equeue_call(&q, simple_func, &touched);
equeue_cancel(&q, id);
test_assert(equeue_cancel(&q, id));

equeue_dispatch(&q, 0);
test_assert(!touched);
Expand Down Expand Up @@ -352,19 +352,19 @@ void cancel_unnecessarily_test(void)

int id = equeue_call(&q, pass_func, 0);
for (int i = 0; i < 5; i++) {
equeue_cancel(&q, id);
test_assert(equeue_cancel(&q, id) == (i == 0));
}

id = equeue_call(&q, pass_func, 0);
equeue_dispatch(&q, 0);
for (int i = 0; i < 5; i++) {
equeue_cancel(&q, id);
test_assert(!equeue_cancel(&q, id));
}

bool touched = false;
equeue_call(&q, simple_func, &touched);
for (int i = 0; i < 5; i++) {
equeue_cancel(&q, id);
test_assert(!equeue_cancel(&q, id));
}

equeue_dispatch(&q, 0);
Expand Down Expand Up @@ -595,8 +595,8 @@ void chain_test(void)
id2 = equeue_call_in(&q2, 5, simple_func, &touched);
test_assert(id1 && id2);

equeue_cancel(&q1, id1);
equeue_cancel(&q2, id2);
test_assert(equeue_cancel(&q1, id1));
test_assert(equeue_cancel(&q2, id2));

id1 = equeue_call_in(&q1, 10, simple_func, &touched);
id2 = equeue_call_in(&q2, 10, simple_func, &touched);
Expand Down Expand Up @@ -768,7 +768,7 @@ void break_request_cleared_on_timeout(void)
equeue_dispatch(&q, 10);
test_assert(pq.p == 1);

equeue_cancel(&q, id);
test_assert(equeue_cancel(&q, id));

int count = 0;
equeue_call_every(&q, 10, simple_func, &count);
Expand Down Expand Up @@ -796,9 +796,9 @@ void sibling_test(void)
test_assert(!s->next);
}
}
equeue_cancel(&q, id0);
equeue_cancel(&q, id1);
equeue_cancel(&q, id2);
test_assert(equeue_cancel(&q, id0));
test_assert(equeue_cancel(&q, id1));
test_assert(equeue_cancel(&q, id2));
equeue_destroy(&q);
}

Expand Down Expand Up @@ -829,7 +829,7 @@ void user_allocated_event_test()
equeue_post_user_allocated(&q, simple_func, &e3.e);
equeue_post_user_allocated(&q, simple_func, &e4.e);
equeue_post_user_allocated(&q, simple_func, &e5.e);
equeue_cancel_user_allocated(&q, &e3.e);
test_assert(equeue_cancel_user_allocated(&q, &e3.e));

equeue_dispatch(&q, 1);

Expand All @@ -852,6 +852,21 @@ void user_allocated_event_test()
equeue_destroy(&q);
}

void id_cycle()
{
equeue_t q;
int err = equeue_create(&q, 10000000);
test_assert(!err);

for (int i = 0; i < 300; i++) {
int id = equeue_call(&q, pass_func, 0);
test_assert(id != 0);
test_assert(equeue_cancel(&q, id));
}

equeue_destroy(&q);
}

int main()
{
printf("beginning tests...\n");
Expand Down Expand Up @@ -881,6 +896,7 @@ int main()
test_run(break_request_cleared_on_timeout);
test_run(sibling_test);
test_run(user_allocated_event_test);
test_run(id_cycle);
printf("done!\n");
return test_failure;
}