"Chandra Pratap via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes:
> +static int test_prio_queue(int *input, int *result)
> +{
> + struct prio_queue pq = { intcmp };
> + int i = 0;
> +
> + while (*input) {
> + int *val = input++;
> + void *peek, *get;
> + switch(*val) {
Ah, this one is a bit tricky. I would have expected that we can
make val a pure integer, but because we need to give the address of
the element to be placed in the queue, not the value to be placed as
an element in the queue, to prio_queue_put(), we would need the
pointer.
I wonder if writing this as a more conventional for(;;) loop would
make it easier to handle, i.e.
int val, i;
for (i = 0; (val = *input); input++) {
switch (val) {
case ...:
...
default:
prio_queue_put(&pq, input);
break;
}
}
> +...
> + default:
> + prio_queue_put(&pq, val);
> + break;
> + }
> + }
> + clear_prio_queue(&pq);
> + return 0;
> +}
> +#define TEST_INPUT(INPUT, EXPECTED, name) \
> + static void test_##name(void) \
> +{ \
> + int input[] = {INPUT}; \
I think I missed this myself in my review the last round, but we
need the sentinel value 0 at the end, i.e.,
int input[] = {INPUT, 0};
because the test_prio_queue() loops "while (*input)". Otherwise
the loop would not terminate.
> + int expected[] = {EXPECTED}; \
> + int result[ARRAY_SIZE(expected)]; \
These arrays are correct.
> + test_prio_queue(input, result); \
> + check(!memcmp(expected, result, sizeof(expected))); \
So is this check.
> +}
> +
> +TEST_INPUT(BASIC_INPUT, BASIC_EXPECTED, basic)
> +TEST_INPUT(MIXED_PUT_GET_INPUT, MIXED_PUT_GET_EXPECTED, mixed)
> +TEST_INPUT(EMPTY_QUEUE_INPUT, EMPTY_QUEUE_EXPECTED, empty)
> +TEST_INPUT(STACK_INPUT, STACK_EXPECTED, stack)
> +TEST_INPUT(REVERSE_STACK_INPUT, REVERSE_STACK_EXPECTED, reverse)
> +
> +int cmd_main(int argc, const char **argv)
> +{
> + TEST(test_basic(), "prio-queue works for basic input");
> + TEST(test_mixed(), "prio-queue works for mixed put & get commands");
> + TEST(test_empty(), "prio-queue works when queue is empty");
> + TEST(test_stack(), "prio-queue works when used as a LIFO stack");
> + TEST(test_reverse(), "prio-queue works when LIFO stack is reversed");
> +
> + return test_done();
> +}
Other than that, looking good. Thanks.
