Skip to content

Commit

Permalink
Fix negative modulo for C89 (#45)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
bkthomps authored May 21, 2019
1 parent b07e06d commit 411af13
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 6 deletions.
2 changes: 1 addition & 1 deletion CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
17 changes: 12 additions & 5 deletions src/deque.c
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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;
Expand Down
20 changes: 20 additions & 0 deletions tst/deque.c
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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();
}

0 comments on commit 411af13

Please sign in to comment.