The intuitive meaning of "missing" would be the difference between
tlength and the current queue length, and that's how memblockq-test
assumed pa_memblockq_pop_missing() to define the term "missing", but
that was an incorrect assumption, causing the last
pa_memblockq_pop_missing() return value assertion to fail.
This patch fixes the failing assertion and adds some comments about how
the "missing" and "requested" variables in memblockq work.
---
src/tests/memblockq-test.c | 95 ++++++++++++++++++++++++++++++++++++----------
1 file changed, 75 insertions(+), 20 deletions(-)
diff --git a/src/tests/memblockq-test.c b/src/tests/memblockq-test.c
index fc83d99..2a9b88a 100644
--- a/src/tests/memblockq-test.c
+++ b/src/tests/memblockq-test.c
@@ -421,74 +421,129 @@ START_TEST (memblockq_test_pop_missing) {
bq = pa_memblockq_new("test memblockq", idx, maxlength, tlength, &ss, prebuf, minreq, maxrewind, &silence);
fail_unless(bq != NULL);
- /* initially, the whole target length of bytes is missing */
+ /* The following equation regarding the internal variables of a memblockq
+ * is always true:
+ *
+ * length + missing + requested = tlength
+ *
+ * "length" is the current memblockq length (write index minus read index)
+ * and "tlength" is the target length. The intuitive meaning of "missing"
+ * would be the difference between tlength and length, but actually
+ * "missing" and "requested" together constitute the amount that is missing
+ * from the queue. Writing to the queue decrements "requested" and reading
+ * from the queue increments "missing". pa_memblockq_pop_missing() resets
+ * "missing" to zero, returns the old "missing" value and adds the
+ * equivalent amount to "requested".
+ *
+ * This test has comments between each step documenting the assumed state
+ * of those internal variables. */
+
+ /* length + missing + requested = tlength
+ * 0 + 100 + 0 = 100 */
+
ck_assert_int_eq(pa_memblockq_pop_missing(bq), tlength);
- /* add 20 bytes of data */
+ /* length + missing + requested = tlength
+ * 0 + 0 + 100 = 100 */
+
for (int i = 0; i != 2; ++i)
ck_assert_int_eq(pa_memblockq_push(bq, &data), 0);
check_queue_invariants(bq);
- /* no new missing data is reported */
+ /* length + missing + requested = tlength
+ * 20 + 0 + 80 = 100 */
+
ck_assert_int_eq(pa_memblockq_pop_missing(bq), 0);
- /* fill up to 100 bytes of data */
+ /* length + missing + requested = tlength
+ * 20 + 0 + 80 = 100 */
+
for (int i = 0; i != 8; ++i)
ck_assert_int_eq(pa_memblockq_push(bq, &data), 0);
check_queue_invariants(bq);
- /* queue fill level is at target level now */
+ /* length + missing + requested = tlength
+ * 100 + 0 + 0 = 100 */
+
ck_assert_int_eq(pa_memblockq_pop_missing(bq), 0);
- /* pop 40 bytes of data, down to 60 bytes fill level */
+ /* length + missing + requested = tlength
+ * 100 + 0 + 0 = 100 */
+
ck_assert_int_eq(pa_memblockq_peek_fixed_size(bq, 40, &chunk), 0);
pa_memblockq_drop(bq, 40);
ck_assert_int_eq(chunk.length - chunk.index, 40);
pa_memblock_unref(chunk.memblock);
check_queue_invariants(bq);
- /* queue fill level is 40 bytes under target length
- * This is less than minreq, so no missing data is reported */
+ /* length + missing + requested = tlength
+ * 60 + 40 + 0 = 100 */
+
+ /* 40 bytes are missing, but that's less than minreq, so 0 is reported */
ck_assert_int_eq(pa_memblockq_pop_missing(bq), 0);
- /* add 30 bytes of data, up to 90 bytes fill level */
+ /* length + missing + requested = tlength
+ * 60 + 40 + 0 = 100 */
+
+ /* Now we push 30 bytes even though it was not requested, so the requested
+ * counter goes negative! */
for (int i = 0; i != 3; ++i)
ck_assert_int_eq(pa_memblockq_push(bq, &data), 0);
check_queue_invariants(bq);
- /* queue fill level is 10 bytes under target length
- * This is less than minreq, so no missing data is reported. */
+ /* length + missing + requested = tlength
+ * 90 + 40 + -30 = 100 */
+
+ /* missing < minreq, so nothing is reported missing. */
ck_assert_int_eq(pa_memblockq_pop_missing(bq), 0);
- /* pop 20 bytes of data, down to 70 bytes of data */
+ /* length + missing + requested = tlength
+ * 90 + 40 + -30 = 100 */
+
ck_assert_int_eq(pa_memblockq_peek_fixed_size(bq, 20, &chunk), 0);
pa_memblockq_drop(bq, 20);
ck_assert_int_eq(chunk.length - chunk.index, 20);
pa_memblock_unref(chunk.memblock);
check_queue_invariants(bq);
- /* queue fill level is 30 bytes under target length
- * This is less than minreq, so no missing data is reported */
+ /* length + missing + requested = tlength
+ * 70 + 60 + -30 = 100 */
+
+ /* missing < minreq, so nothing is reported missing. */
ck_assert_int_eq(pa_memblockq_pop_missing(bq), 0);
- /* add 50 bytes of data, up to 120 bytes fill level */
+ /* length + missing + requested = tlength
+ * 70 + 60 + -30 = 100 */
+
+ /* We push more data again even though it was not requested, so the
+ * requested counter goes further into the negative range. */
for (int i = 0; i != 5; ++i)
ck_assert_int_eq(pa_memblockq_push(bq, &data), 0);
check_queue_invariants(bq);
- /* queue fill level is above target level, so no missing data is reported. */
+ /* length + missing + requested = tlength
+ * 120 + 60 + -80 = 100 */
+
+ /* missing < minreq, so nothing is reported missing. */
ck_assert_int_eq(pa_memblockq_pop_missing(bq), 0);
- /* pop 20 bytes of data, down the target level */
+ /* length + missing + requested = tlength
+ * 120 + 60 + -80 = 100 */
+
ck_assert_int_eq(pa_memblockq_peek_fixed_size(bq, 20, &chunk), 0);
pa_memblockq_drop(bq, 20);
ck_assert_int_eq(chunk.length - chunk.index, 20);
pa_memblock_unref(chunk.memblock);
check_queue_invariants(bq);
- /* queue fill level is at target level now
- * No missing data should be reported. */
- ck_assert_int_eq(pa_memblockq_pop_missing(bq), 0);
+ /* length + missing + requested = tlength
+ * 100 + 80 + -80 = 100 */
+
+ /* missing has now reached the minreq threshold */
+ ck_assert_int_eq(pa_memblockq_pop_missing(bq), 80);
+
+ /* length + missing + requested = tlength
+ * 100 + 0 + 0 = 100 */
/* cleanup */
pa_memblockq_free(bq);
--
2.10.2
