From 411af131c2972c124fdb4b3f00460dcdce6c3974 Mon Sep 17 00:00:00 2001 From: Bailey Thompson Date: Tue, 21 May 2019 17:46:53 -0400 Subject: [PATCH] Fix negative modulo for C89 (#45) In C89, negative integer division and modulo is implementation-defined. Case 1: division rounds to 0 (C89/90, C99, C11, C18) -9 / 7 -> -1 -9 % 7 -> -2 Case 2: division rounds to negative infinity (C89/90) -9 / 7 -> -2 -9 % 7 -> 5 This change fixes the deque data structure from having negative division and modulo to prevent this implementation-defined behavior. --- CMakeLists.txt | 2 +- src/deque.c | 17 ++++++++++++----- tst/deque.c | 20 ++++++++++++++++++++ 3 files changed, 33 insertions(+), 6 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 6eeb4167..051713a7 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -3,7 +3,7 @@ project(Containers C) set(CMAKE_C_STANDARD 90) -set(CMAKE_C_FLAGS "-pedantic -ldl -g -O0 -Wall -fprofile-arcs -ftest-coverage") +set(CMAKE_C_FLAGS "-Wall -Wextra -Wpedantic -ldl -g -O0 -fprofile-arcs -ftest-coverage") add_executable(Containers tst/test.c tst/test.h src/array.c src/include/array.h tst/array.c diff --git a/src/deque.c b/src/deque.c index 79bc6c6d..6e7a73a7 100644 --- a/src/deque.c +++ b/src/deque.c @@ -114,8 +114,12 @@ int deque_is_empty(deque me) int deque_trim(deque me) { int i; - const int start_block = me->start_index / BLOCK_SIZE; - const int end_block = (me->end_index - 1) / BLOCK_SIZE; + /* The start and end blocks are written like this because in C89, */ + /* negative integer division and modulo are implementation-defined. */ + const int start_block = + me->start_index == -1 ? 0 : me->start_index / BLOCK_SIZE; + const int end_block = + me->end_index == 0 ? 0 : (me->end_index - 1) / BLOCK_SIZE; const int new_block_count = end_block - start_block + 1; void *const new_block = malloc(new_block_count * sizeof(struct node)); if (!new_block) { @@ -166,11 +170,14 @@ void deque_copy_to_array(void *const arr, deque me) int deque_push_front(deque me, void *const data) { struct node block_item; - int block_index = me->start_index / BLOCK_SIZE; - int inner_index = me->start_index % BLOCK_SIZE; - if (inner_index == -1) { + int block_index; + int inner_index; + if (me->start_index == -1) { block_index = -1; inner_index = BLOCK_SIZE - 1; + } else { + block_index = me->start_index / BLOCK_SIZE; + inner_index = me->start_index % BLOCK_SIZE; } if (inner_index == BLOCK_SIZE - 1) { struct node *block_item_reference; diff --git a/tst/deque.c b/tst/deque.c index 5d75d5d0..4f7f66a8 100644 --- a/tst/deque.c +++ b/tst/deque.c @@ -253,6 +253,25 @@ static void test_clear_out_of_memory(void) assert(!deque_destroy(me)); } +void test_single_full_block(void) +{ + int i; + int num = 5; + deque me = deque_init(sizeof(int)); + for (i = 0; i < 5; i++) { + deque_push_front(me, &num); + } + for (i = 0; i < 3; i++) { + deque_push_back(me, &num); + } + for (i = 0; i < 8; i++) { + deque_pop_back(&num, me); + } + deque_trim(me); + assert(deque_size(me) == 0); + deque_destroy(me); +} + void test_deque(void) { test_invalid_init(); @@ -263,4 +282,5 @@ void test_deque(void) test_push_front_out_of_memory(); test_push_back_out_of_memory(); test_clear_out_of_memory(); + test_single_full_block(); }