From 0b8cc2fcf3da70539bb545eb6e491f512aa82a41 Mon Sep 17 00:00:00 2001 From: Alex Young Date: Sat, 9 May 2015 14:46:40 +0100 Subject: [PATCH] new interface, tests, examples, readme, threadsafe - updated tests to test new interface - corrected bugs found in tests - changed what is and isn't classed as an error - updated example - updated readme --- README.md | 45 ++++-- examples/simple.c | 3 +- src/queue.h | 84 ++++++------ tests/check_queue.c | 323 +++++++++++++++++--------------------------- 4 files changed, 200 insertions(+), 255 deletions(-) diff --git a/README.md b/README.md index ad55bf5..730e373 100644 --- a/README.md +++ b/README.md @@ -1,25 +1,26 @@ # queue -A type-agnostic header-only macro-based struct queue implementation in C. +A thread safe type-agnostic header-only macro-based structure queue +implementation in C. ## Usage -To create a queueable struct, include the `queue_handle qh` member in your -struct. +Include the queue.h header file in your source. To create a queueable +structure, include the `queue_handle qh` member in the struct definition. -Before using QUEUE_PUSH, ensure that the queue struct is set to NULL, this is -the only initialisation that needs to be done. - -This queue implementation is not thread safe. +Before any other operations, ensure that QUEUE_INIT has been called, it is an +error to not do so. This initialises the queue and sets up the mutex and +conditions variables. Setting the queue to NULL will protect against errors +if a queue has not been Freeing the queue does not free every element in the queue if they have been dynamically allocated. This has to be done by popping all the elements in the queue and freeing them manually. -Pushing the same element (at the same address) into the queue is not supported. -This is because the `next` member of the queue's `queue_handle` will be -overwitten. This will case undefined behaviour when pushing or popping to or -from the queue (see commented out test case). +Pushing the same element (at the same address) into the queue is not supported +and is an error. This is because the `next` member of the queue's `queue_handle` +will be overwritten. This will cause undefined behavior when pushing or popping +to or from the queue (see commented out test case). ```c #include @@ -35,7 +36,7 @@ int main(void) { struct msg *msgs; // message queue struct msg m1, *m2; - msgs = NULL; + QUEUE_INIT(struct msg, msgs); m1.content = "abc"; QUEUE_PUSH(msgs, &m1); @@ -50,3 +51,23 @@ int main(void) { return EXIT_SUCCESS; } ``` + +## Testing and building examples + +To run tests run + +```bash +make check +``` + +To compile the examples run + +```bash +make examples +``` + +Both of the above can be done together by running + +```bash +make +``` diff --git a/examples/simple.c b/examples/simple.c index 234a27b..0798527 100644 --- a/examples/simple.c +++ b/examples/simple.c @@ -11,8 +11,7 @@ int main(void) { struct msg *msgs; // message queue struct msg m1, *m2; - msgs = malloc(sizeof (struct msg)); - QUEUE_INIT(msgs); + QUEUE_INIT(struct msg, msgs); m1.content = "abc"; QUEUE_PUSH(msgs, &m1); diff --git a/src/queue.h b/src/queue.h index 1e86fa6..74c6e85 100644 --- a/src/queue.h +++ b/src/queue.h @@ -18,63 +18,68 @@ typedef struct queue_handle { void *next; } queue_handle; -#define QUEUE_INIT(q) \ +#define QUEUE_INIT(qt, q) \ do { \ - (q)->qh.qc = malloc(sizeof (queue_core)); \ - queue_core *qc = (q)->qh.qc; \ - pthread_mutex_init(&qc->mutex, NULL); \ - pthread_cond_init(&qc->cond, NULL); \ - qc->front = qc->back = NULL; \ - qc->backqh = NULL; \ - qc->size = 0; \ - (q)->qh.next = NULL; \ + (q) = malloc(sizeof (qt)); \ + if (q) { \ + (q)->qh.qc = malloc(sizeof (queue_core)); \ + if ((q)->qh.qc) { \ + queue_core *qc = (q)->qh.qc; \ + pthread_mutex_init(&qc->mutex, NULL); \ + pthread_cond_init(&qc->cond, NULL); \ + qc->front = qc->back = NULL; \ + qc->backqh = NULL; \ + qc->size = 0; \ + (q)->qh.next = NULL; \ + } else { \ + free(q); \ + (q) = NULL; \ + } \ + } \ } while (0) #define QUEUE_PUSH(q, e) \ do { \ - if (q && (q)->qh.qc) { \ - queue_core *qc = (q)->qh.qc; \ - queue_handle *backqh; \ - pthread_mutex_lock(&qc->mutex); \ - (e)->qh.qc = qc; \ - (e)->qh.next = NULL; \ - backqh = qc->backqh; \ - if (!qc->front) { /* empty queue */ \ - qc->front = qc->back = (e); \ - } else { /* non-empty queue */ \ - backqh->next = (e); \ - qc->back = (e); \ - } \ - backqh = &(e)->qh; \ - qc->size++; \ - pthread_mutex_unlock(&qc->mutex); \ - pthread_cond_signal(&qc->cond); /* broadcast to all? */ \ + queue_core *qc = (q)->qh.qc; \ + queue_handle *backqh; \ + pthread_mutex_lock(&qc->mutex); \ + (e)->qh.qc = qc; \ + (e)->qh.next = NULL; \ + backqh = qc->backqh; \ + if (!qc->front) { /* empty queue */ \ + qc->front = qc->back = (e); \ + } else { /* non-empty queue */ \ + backqh->next = (e); \ + qc->back = (e); \ } \ + backqh = &((e)->qh); \ + qc->backqh = backqh; \ + qc->size++; \ + pthread_mutex_unlock(&qc->mutex); \ + pthread_cond_signal(&qc->cond); /* broadcast to all? */ \ } while (0) #define QUEUE_POP(q, e) \ do { \ (e) = NULL; \ - if (q && (q)->qh.qc) { \ - queue_core *qc = (q)->qh.qc; \ - pthread_mutex_lock(&qc->mutex); \ - while (QUEUE_SIZE(q) == 0) { \ - pthread_cond_wait(&qc->cond, &qc->mutex); \ - } \ - if ((q)->qh.qc->front != NULL) { \ - (e) = (q)->qh.qc->front; \ - (q)->qh.qc->front = (e)->qh.next; \ - (q)->qh.qc->size--; \ - } \ - pthread_mutex_unlock(&qc->mutex); \ + queue_core *qc = (q)->qh.qc; \ + pthread_mutex_lock(&qc->mutex); \ + while (QUEUE_SIZE(q) == 0) { \ + pthread_cond_wait(&qc->cond, &qc->mutex); \ + } \ + if ((q)->qh.qc->front != NULL) { \ + (e) = (q)->qh.qc->front; \ + (q)->qh.qc->front = (e)->qh.next; \ + (q)->qh.qc->size--; \ } \ + pthread_mutex_unlock(&qc->mutex); \ } while (0) #define QUEUE_LOCK(q) \ pthread_mutex_lock(&q->qh.qc->mutex); #define QUEUE_SIZE(q) \ - ((q) ? (q)->qh.qc->size : 0U) + ((q)->qh.qc->size) #define QUEUE_UNLOCK(q) \ pthread_mutex_unlock(&q->qh.qc->mutex); @@ -86,6 +91,7 @@ typedef struct queue_handle { pthread_cond_destroy(&qc->cond); \ pthread_mutex_destroy(&qc->mutex); \ free(qc); \ + free(q); \ } \ } while (0) diff --git a/tests/check_queue.c b/tests/check_queue.c index 00e54e7..7102d25 100644 --- a/tests/check_queue.c +++ b/tests/check_queue.c @@ -7,259 +7,183 @@ struct msg { queue_handle qh; }; -START_TEST(test_queue_size_null) { - struct msg *msgs; // message queue +START_TEST(test_queue_init) { + struct msg *msgs; - msgs = NULL; + QUEUE_INIT(struct msg, msgs); - ck_assert(msgs == NULL); - ck_assert_int_eq(QUEUE_SIZE(msgs), 0); -} END_TEST - -START_TEST(test_queue_free_null) { - struct msg *msgs; // message queue - - QUEUE_FREE(msgs); -} END_TEST - -START_TEST(test_queue_push) { - struct msg *msgs; // message queue - struct msg m; - - m.content = "abc"; - - QUEUE_PUSH(msgs, &m); - - ck_assert(msgs != NULL); - - ck_assert(msgs->qh.q == m.qh.q); - - ck_assert(msgs->qh.q->front == &m); - ck_assert(msgs->qh.q->front == msgs->qh.q->back); - - ck_assert(msgs->qh.q->backqh == &((struct msg *)msgs->qh.q->back)->qh); - - ck_assert_int_eq(msgs->qh.q->size, 1); - ck_assert(msgs->qh.next == NULL); - - QUEUE_FREE(msgs); -} END_TEST + ck_assert_ptr_ne(msgs, NULL); + ck_assert_ptr_ne(msgs->qh.qc, NULL); -START_TEST(test_queue_size) { - struct msg *msgs; // message queue - struct msg m1, m2; + ck_assert_ptr_eq(msgs->qh.qc->front, NULL); + ck_assert_ptr_eq(msgs->qh.qc->back, NULL); + ck_assert_ptr_eq(msgs->qh.qc->backqh, NULL); - m1.content = "abc"; - m2.content = "def"; + ck_assert_uint_eq(msgs->qh.qc->size, 0); - QUEUE_PUSH(msgs, &m1); - ck_assert_int_eq(QUEUE_SIZE(msgs), 1); - QUEUE_PUSH(msgs, &m2); - ck_assert_int_eq(QUEUE_SIZE(msgs), 2); + ck_assert_ptr_eq(msgs->qh.next, NULL); QUEUE_FREE(msgs); } END_TEST -START_TEST(test_queue_push_2) { - struct msg *msgs; // message queue - struct msg m1, *m2; +START_TEST(test_queue_push) { + struct msg *msgs; + struct msg m1, m2, m3; - m2 = malloc(sizeof (struct msg)); + QUEUE_INIT(struct msg, msgs); m1.content = "abc"; - m2->content = "def"; + m2.content = "def"; + m3.content = "ghi"; QUEUE_PUSH(msgs, &m1); - ck_assert_int_eq(QUEUE_SIZE(msgs), 1); - QUEUE_PUSH(msgs, m2); - ck_assert_int_eq(QUEUE_SIZE(msgs), 2); - ck_assert(msgs->qh.q == m1.qh.q); - ck_assert(msgs->qh.q == m2->qh.q); + ck_assert_ptr_eq(msgs->qh.qc, m1.qh.qc); - ck_assert(msgs->qh.q->front == &m1); + ck_assert_ptr_eq(msgs->qh.qc->front, &m1); + ck_assert_ptr_eq(msgs->qh.qc->back, &m1); + ck_assert_ptr_eq(msgs->qh.qc->backqh, &m1.qh); + ck_assert_uint_eq(msgs->qh.qc->size, 1); + ck_assert_ptr_eq(msgs->qh.next, NULL); - ck_assert(m1.qh.next == m2); - ck_assert(m2->qh.next == NULL); + QUEUE_PUSH(msgs, &m2); - ck_assert(msgs->qh.q->back == m2); - ck_assert(msgs->qh.q->backqh == &m2->qh); + ck_assert_ptr_eq(msgs->qh.qc, m2.qh.qc); + ck_assert_ptr_eq(msgs->qh.qc->front, &m1); + ck_assert_ptr_eq(msgs->qh.qc->back, &m2); + ck_assert_ptr_eq(msgs->qh.qc->backqh, &m2.qh); + ck_assert_uint_eq(msgs->qh.qc->size, 2); + ck_assert_ptr_eq(m1.qh.next, &m2); + ck_assert_ptr_eq(m2.qh.next, NULL); + + QUEUE_PUSH(msgs, &m3); + + ck_assert_ptr_eq(msgs->qh.qc, m3.qh.qc); + ck_assert_ptr_eq(msgs->qh.qc->front, &m1); + ck_assert_ptr_eq(msgs->qh.qc->back, &m3); + ck_assert_ptr_eq(msgs->qh.qc->backqh, &m3.qh); + ck_assert_uint_eq(msgs->qh.qc->size, 3); + ck_assert_ptr_eq(m1.qh.next, &m2); + ck_assert_ptr_eq(m2.qh.next, &m3); + ck_assert_ptr_eq(m3.qh.next, NULL); QUEUE_FREE(msgs); - - free(m2); } END_TEST -START_TEST(test_queue_push_5) { - struct msg *msgs; // message queue - struct msg m1, *m2, *m3, m4, m5; +START_TEST(test_queue_size) { + struct msg *msgs; + struct msg m1, m2, m3; - m2 = malloc(sizeof (struct msg)); - m3 = malloc(sizeof (struct msg)); + QUEUE_INIT(struct msg, msgs); m1.content = "abc"; - m2->content = "def"; - m3->content = "ghi"; - m4.content = "jkl"; - m5.content = "mno"; - - QUEUE_PUSH(msgs, &m1); - ck_assert_int_eq(QUEUE_SIZE(msgs), 1); - QUEUE_PUSH(msgs, m2); - ck_assert_int_eq(QUEUE_SIZE(msgs), 2); - QUEUE_PUSH(msgs, m3); - ck_assert_int_eq(QUEUE_SIZE(msgs), 3); - QUEUE_PUSH(msgs, &m4); - ck_assert_int_eq(QUEUE_SIZE(msgs), 4); - QUEUE_PUSH(msgs, &m5); - ck_assert_int_eq(QUEUE_SIZE(msgs), 5); - - ck_assert(msgs->qh.q == m1.qh.q); - ck_assert(msgs->qh.q == m2->qh.q); - ck_assert(msgs->qh.q == m3->qh.q); - ck_assert(msgs->qh.q == m4.qh.q); - ck_assert(msgs->qh.q == m5.qh.q); - - ck_assert(msgs->qh.q->front == &m1); - - ck_assert(m1.qh.next == m2); - ck_assert(m2->qh.next == m3); - ck_assert(m3->qh.next == &m4); - ck_assert(m4.qh.next == &m5); - ck_assert(m5.qh.next == NULL); - - ck_assert(msgs->qh.q->back == &m5); - ck_assert(msgs->qh.q->backqh == &m5.qh); - - QUEUE_FREE(msgs); + m2.content = "def"; + m3.content = "ghi"; - free(m2); - free(m3); -} END_TEST + ck_assert_uint_eq(QUEUE_SIZE(msgs), 0); -START_TEST(test_queue_pop_empty) { - struct msg *msgs; // message queue - struct msg *dst; + QUEUE_PUSH(msgs, &m1); + ck_assert_uint_eq(QUEUE_SIZE(msgs), 1); - QUEUE_POP(msgs, dst); - ck_assert(dst == NULL); - ck_assert_int_eq(QUEUE_SIZE(msgs), 0); + QUEUE_PUSH(msgs, &m2); + ck_assert_uint_eq(QUEUE_SIZE(msgs), 2); - QUEUE_FREE(msgs); + QUEUE_PUSH(msgs, &m3); + ck_assert_uint_eq(QUEUE_SIZE(msgs), 3); } END_TEST START_TEST(test_queue_pop) { - struct msg *msgs; // message queue - struct msg src, *dst; - - src.content = "abc"; - - QUEUE_PUSH(msgs, &src); - ck_assert_int_eq(QUEUE_SIZE(msgs), 1); - - QUEUE_POP(msgs, dst); + struct msg *msgs; + struct msg src1, src2, src3, *dst1, *dst2, *dst3; - ck_assert(msgs->qh.q->front == NULL); - - ck_assert(dst == &src); - ck_assert_str_eq(dst->content, src.content); - ck_assert(&dst->qh == &src.qh); - - ck_assert_int_eq(QUEUE_SIZE(msgs), 0); - - QUEUE_FREE(msgs); -} END_TEST - -START_TEST(test_queue_push_pop_push_pop) { - struct msg *msgs; // message queue - struct msg src1, src2, *dst1, *dst2; + QUEUE_INIT(struct msg, msgs); src1.content = "abc"; + src2.content = "def"; + src3.content = "ghi"; QUEUE_PUSH(msgs, &src1); + QUEUE_POP(msgs, dst1); + ck_assert_ptr_eq(dst1, &src1); + ck_assert_ptr_eq(msgs->qh.qc->front, NULL); + ck_assert_ptr_eq(msgs->qh.qc->back, &src1); // unchanged w/ pop + ck_assert_ptr_eq(msgs->qh.qc->backqh, &src1.qh); // unchanged w/ pop + ck_assert_uint_eq(msgs->qh.qc->size, 0); + ck_assert_ptr_eq(dst1->qh.next, msgs->qh.qc->front); + ck_assert_str_eq(src1.content, dst1->content); + + QUEUE_PUSH(msgs, &src1); QUEUE_PUSH(msgs, &src2); + QUEUE_POP(msgs, dst1); + ck_assert_ptr_eq(dst1, &src1); + ck_assert_ptr_eq(msgs->qh.qc->front, &src2); + ck_assert_ptr_eq(msgs->qh.qc->back, &src2); // unchanged w/ pop + ck_assert_ptr_eq(msgs->qh.qc->backqh, &src2.qh); // unchanged w/ pop + ck_assert_uint_eq(msgs->qh.qc->size, 1); + ck_assert_ptr_eq(dst1->qh.next, msgs->qh.qc->front); + ck_assert_str_eq(src1.content, dst1->content); QUEUE_POP(msgs, dst2); - - ck_assert(dst1 == &src1); - ck_assert(dst2 == &src2); - - ck_assert_int_eq(QUEUE_SIZE(msgs), 0); - - QUEUE_FREE(msgs); -} END_TEST - -START_TEST(test_queue_pop_5) { - struct msg *msgs; // message queue - struct msg src1, *src2, *src3, src4, src5; - struct msg *dst1, *dst2, *dst3, *dst4, *dst5; - - src2 = malloc(sizeof (struct msg)); - src3 = malloc(sizeof (struct msg)); - - src1.content = "abc"; - src2->content = "def"; - src3->content = "ghi"; - src4.content = "jkl"; - src5.content = "mno"; + ck_assert_ptr_eq(dst2, &src2); + ck_assert_ptr_eq(msgs->qh.qc->front, NULL); + ck_assert_ptr_eq(msgs->qh.qc->back, &src2); // unchanged w/ pop + ck_assert_ptr_eq(msgs->qh.qc->backqh, &src2.qh); // unchanged w/ pop + ck_assert_uint_eq(msgs->qh.qc->size, 0); + ck_assert_ptr_eq(dst2->qh.next, msgs->qh.qc->front); + ck_assert_str_eq(src2.content, dst2->content); QUEUE_PUSH(msgs, &src1); - QUEUE_PUSH(msgs, src2); - QUEUE_PUSH(msgs, src3); - QUEUE_PUSH(msgs, &src4); - QUEUE_PUSH(msgs, &src5); - ck_assert_int_eq(QUEUE_SIZE(msgs), 5); - ck_assert(msgs->qh.q->front == &src1); - + QUEUE_PUSH(msgs, &src2); + QUEUE_PUSH(msgs, &src3); QUEUE_POP(msgs, dst1); - ck_assert_int_eq(QUEUE_SIZE(msgs), 4); - ck_assert(dst1 == &src1); - ck_assert(msgs->qh.q->front == src2); - + ck_assert_ptr_eq(dst1, &src1); + ck_assert_ptr_eq(msgs->qh.qc->front, &src2); + ck_assert_ptr_eq(msgs->qh.qc->back, &src3); // unchanged w/ pop + ck_assert_ptr_eq(msgs->qh.qc->backqh, &src3.qh); // unchanged w/ pop + ck_assert_uint_eq(msgs->qh.qc->size, 2); + ck_assert_ptr_eq(dst1->qh.next, msgs->qh.qc->front); + ck_assert_str_eq(src1.content, dst1->content); QUEUE_POP(msgs, dst2); - ck_assert_int_eq(QUEUE_SIZE(msgs), 3); - ck_assert(dst2 == src2); - ck_assert(msgs->qh.q->front == src3); - + ck_assert_ptr_eq(dst2, &src2); + ck_assert_ptr_eq(msgs->qh.qc->front, &src3); + ck_assert_ptr_eq(msgs->qh.qc->back, &src3); // unchanged w/ pop + ck_assert_ptr_eq(msgs->qh.qc->backqh, &src3.qh); // unchanged w/ pop + ck_assert_uint_eq(msgs->qh.qc->size, 1); + ck_assert_ptr_eq(dst2->qh.next, msgs->qh.qc->front); + ck_assert_str_eq(src2.content, dst2->content); QUEUE_POP(msgs, dst3); - ck_assert_int_eq(QUEUE_SIZE(msgs), 2); - ck_assert(dst3 == src3); - ck_assert(msgs->qh.q->front == &src4); - - QUEUE_POP(msgs, dst4); - ck_assert_int_eq(QUEUE_SIZE(msgs), 1); - ck_assert(dst4 == &src4); - ck_assert(msgs->qh.q->front == &src5); - - QUEUE_POP(msgs, dst5); - ck_assert_int_eq(QUEUE_SIZE(msgs), 0); - ck_assert(dst5 == &src5); - ck_assert(msgs->qh.q->front == NULL); + ck_assert_ptr_eq(dst3, &src3); + ck_assert_ptr_eq(msgs->qh.qc->front, NULL); + ck_assert_ptr_eq(msgs->qh.qc->back, &src3); // unchanged w/ pop + ck_assert_ptr_eq(msgs->qh.qc->backqh, &src3.qh); // unchanged w/ pop + ck_assert_uint_eq(msgs->qh.qc->size, 0); + ck_assert_ptr_eq(dst3->qh.next, msgs->qh.qc->front); + ck_assert_str_eq(src3.content, dst3->content); QUEUE_FREE(msgs); } END_TEST /* Expected failure due to 'next' being overwritten -START_TEST(test_queue_push_push_pop_same_element) { - struct msg *msgs; // message queue +START_TEST(test_queue_push_same_element) { + struct msg *msgs; struct msg src1, *src2, *dst; + QUEUE_INIT(struct msg, msgs); + src1.content = "abc"; src2 = &src1; QUEUE_PUSH(msgs, &src1); - ck_assert_int_eq(QUEUE_SIZE(msgs), 1); QUEUE_PUSH(msgs, src2); - ck_assert_int_eq(QUEUE_SIZE(msgs), 2); - ck_assert(msgs->qh.q == src1.qh.q); - ck_assert(msgs->qh.q == src2->qh.q); + ck_assert(msgs->qh.qc == src1.qh.qc); + ck_assert(msgs->qh.qc == src2->qh.qc); - ck_assert(msgs->qh.q->front == &src1); - ck_assert(msgs->qh.q->back == src2); - ck_assert(msgs->qh.q->backqh == &src2->qh); + ck_assert(msgs->qh.qc->front == &src1); + ck_assert(msgs->qh.qc->back == src2); + ck_assert(msgs->qh.qc->backqh == &src2->qh); - ck_assert(src1.qh.next == &src1); + ck_assert(src1.qh.next == src2); ck_assert(src2->qh.next == NULL); QUEUE_FREE(msgs); @@ -272,19 +196,14 @@ Suite *queue_suite(void) { s = suite_create("queue"); - /* Core test case */ + // Core test case tc_core = tcase_create("Core"); - tcase_add_test(tc_core, test_queue_size_null); - tcase_add_test(tc_core, test_queue_free_null); - tcase_add_test(tc_core, test_queue_size); + tcase_add_test(tc_core, test_queue_init); tcase_add_test(tc_core, test_queue_push); - tcase_add_test(tc_core, test_queue_push_2); - tcase_add_test(tc_core, test_queue_push_5); - tcase_add_test(tc_core, test_queue_pop_empty); + tcase_add_test(tc_core, test_queue_size); tcase_add_test(tc_core, test_queue_pop); - tcase_add_test(tc_core, test_queue_pop_5); - //tcase_add_test(tc_core, test_queue_push_push_pop_same_element); + //tcase_add_test(tc_core, test_queue_push_same_element); suite_add_tcase(s, tc_core); return s;