diff --git a/include/cgreen/CMakeLists.txt b/include/cgreen/CMakeLists.txt index 6c0c5262..4142e853 100644 --- a/include/cgreen/CMakeLists.txt +++ b/include/cgreen/CMakeLists.txt @@ -12,6 +12,7 @@ set(cgreen_HDRS cute_reporter.h legacy.h mocks.h + memory.h string_comparison.h reporter.h runner.h diff --git a/include/cgreen/cgreen.h b/include/cgreen/cgreen.h index 68b14c34..7d59bb73 100644 --- a/include/cgreen/cgreen.h +++ b/include/cgreen/cgreen.h @@ -6,5 +6,6 @@ #include #include #include +#include #include #include diff --git a/include/cgreen/constraint_syntax_helpers.h b/include/cgreen/constraint_syntax_helpers.h index 1b202d0d..c718fac4 100644 --- a/include/cgreen/constraint_syntax_helpers.h +++ b/include/cgreen/constraint_syntax_helpers.h @@ -45,6 +45,7 @@ #define will_return_double(value) create_return_double_value_constraint(value) #define will_set_contents_of_parameter(parameter_name, value, size) create_set_parameter_value_constraint(#parameter_name, (intptr_t)value, (size_t)size) +void check_memory_leak(TestReporter *reporter); #ifdef __cplusplus extern "C" { diff --git a/include/cgreen/memory.h b/include/cgreen/memory.h new file mode 100644 index 00000000..a2fd0761 --- /dev/null +++ b/include/cgreen/memory.h @@ -0,0 +1,22 @@ +#ifndef MEMORY_HEADER +#define MEMORY_HEADER + +#ifndef __cplusplus +void* cgreen_malloc(size_t size, const char * filename, int line); +#define malloc(size) cgreen_malloc(size, __FILE__, __LINE__) + +void* cgreen_calloc(size_t num_members, size_t size, const char * filename, int line); +#define calloc(num_members, size) cgreen_calloc(num_members, size, __FILE__, __LINE__) + +void* cgreen_realloc(void* pointer, size_t size, const char * filename, int line); +#define realloc(pointer, size) cgreen_realloc(pointer, size, __FILE__, __LINE__) + +void cgreen_free(void* pointer, const char * filename, int line); +#define free(pointer) cgreen_free(pointer, __FILE__, __LINE__) + +#else +#undef check_memory_leak +#define check_memory_leak(reporter) +#endif + +#endif diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 52fcf088..a9919fce 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -36,6 +36,7 @@ set(cgreen_SRCS constraint_syntax_helpers.c cute_reporter.c cdash_reporter.c + memory.c messaging.c message_formatting.c mocks.c diff --git a/src/memory.c b/src/memory.c index ca5cef1d..6be11942 100644 --- a/src/memory.c +++ b/src/memory.c @@ -1,78 +1,150 @@ #include #include -#include "memory.h" +#include +#include +#include "include/cgreen/memory.h" -#define MEMORY_INCREMENT 1024 +typedef struct MemoryEntry_ { + void * memory_location; + size_t size; + const char* filename; + int line_number; +} MemoryEntry; -struct MemoryPool_ { - void** blocks; - long size; - long space; -}; -static void enlarge(MemoryPool *pool); -static int move_to_front(MemoryPool *pool, void *pointer); -static void move_up_one(void **blocks, long amount); +static MemoryEntry ** allocation_memory = NULL; +static int number_of_memory_entries = 0; -MemoryPool *create_memory_pool() { - MemoryPool *pool = (MemoryPool *)malloc(sizeof(MemoryPool)); - if (pool == NULL) { - return NULL; - } - pool->blocks = (void **)malloc(MEMORY_INCREMENT * sizeof(void *)); - if (pool->blocks == NULL) { - free(pool); - return NULL; - } - pool->size = 0; - pool->space = MEMORY_INCREMENT; - return pool; + +static int find_memory_position(void * pointer); +static void move_memory_back(int position_to_start_on); +static MemoryEntry * get_memory_entry(void * pointer); + +#undef malloc +static void create_allocation_memory(void) { + allocation_memory = malloc(sizeof(MemoryEntry)*100); } +#define malloc cgreen_malloc -void free_memory_pool(MemoryPool *pool) { - long i; - for (i = 0; i < pool->size; i++) { - free(pool->blocks[i]); - } - free(pool->blocks); - free(pool); +#undef malloc +void* cgreen_malloc(size_t size, const char * filename, int line) { + if(allocation_memory == NULL) + create_allocation_memory(); + + void * new_memory_pointer = malloc(size); + MemoryEntry * new_memory_entry = malloc(sizeof(MemoryEntry)); + new_memory_entry->memory_location = new_memory_pointer; + new_memory_entry->size = size; + new_memory_entry->filename = filename; + new_memory_entry->line_number = line; + allocation_memory[number_of_memory_entries] = new_memory_entry; + number_of_memory_entries++; + + return new_memory_pointer; } +#define malloc cgreen_malloc -void *memory_pool_allocate(MemoryPool *pool, size_t bytes) { - enlarge(pool); - return pool->blocks[pool->size++] = malloc(bytes); +void* cgreen_calloc(size_t num_members, size_t size, const char * filename, int line) { + void * allocated_memory = cgreen_malloc(num_members * size, filename, line); + if (allocated_memory) + memset(allocated_memory, 0, num_members * size); + return allocated_memory; } -void *memory_pool_reallocate(MemoryPool *pool, void *pointer, size_t bytes) { - if (! move_to_front(pool, pointer)) { +void* cgreen_realloc(void* pointer, size_t size, const char * filename, int line) { + if (pointer == NULL) + return cgreen_malloc(size, filename, line); + + if (size == 0) { + cgreen_free(pointer, filename, line); return NULL; } - return pool->blocks[0] = realloc(pool->blocks[0], bytes); + + MemoryEntry * memory_position = get_memory_entry(pointer); + + void * new_pointer = cgreen_malloc(size, filename, line); + + size_t actual_size = size; + if (size > memory_position->size) + actual_size = memory_position->size; + + memcpy(new_pointer, pointer, actual_size); + + cgreen_free(pointer, filename, line); + + return new_pointer; } -static void enlarge(MemoryPool *pool) { - if (pool->size == pool->space) { - pool->blocks = (void**)realloc(pool->blocks, (pool->space + MEMORY_INCREMENT) * sizeof(void *)); - pool->space += MEMORY_INCREMENT; + +#undef free +void cgreen_free(void * pointer, const char * filename, const int line) { + va_list null_arguments; + memset(&null_arguments, 0, sizeof(null_arguments)); + if (pointer == NULL) { + + TestReporter *reporter = get_test_reporter(); + reporter->show_fail(reporter, filename, line, "Trying to free memory using a NULL pointer", null_arguments); + send_reporter_exception_notification(reporter); + return; + } + + int memory_position = find_memory_position(pointer); + + if (memory_position == -1) { + char message[100]; + sprintf(message, "Trying to free memory [%p] that was not previously allocated", pointer); + + TestReporter *reporter = get_test_reporter(); + reporter->show_fail(reporter, filename, line, message, null_arguments); + send_reporter_exception_notification(reporter); + return; } + + free(allocation_memory[memory_position]); + move_memory_back(memory_position); + number_of_memory_entries--; } +#define free cgreen_free -static int move_to_front(MemoryPool *pool, void *pointer) { - long i; - for (i = 1; i < pool->size; i++) { - if (pool->blocks[i] == pointer) { - move_up_one(pool->blocks, i); - pool->blocks[0] = pointer; - return 1; +void check_memory_leak(TestReporter *reporter) { + if (number_of_memory_entries > 0) { + for( int i = 0 ; i < number_of_memory_entries ; i++) { + MemoryEntry * memoryEntry = allocation_memory[i]; + (reporter->assert_true)( + reporter, + memoryEntry->filename, + memoryEntry->line_number, + 0, + "Memory allocated [%p] was not freed [%s:%d]", + memoryEntry->memory_location, + memoryEntry->filename, + memoryEntry->line_number + ); } } - return 0; } -/* static void move_up_one(void **blocks, long amount) { */ -/* if (amount == 0) { */ -/* return; */ -/* } */ -/* // FIXME: this doesn't compile and the semantics might be wrong */ -/* //memmove(blocks[0], blocks[1], (size_t)(blocks[amount] - blocks[1])); */ -/* } */ +int find_memory_position(void * pointer) { + for( int i = 0 ; i < number_of_memory_entries ; i++) { + MemoryEntry * memoryEntry = allocation_memory[i]; + if(memoryEntry && memoryEntry->memory_location == pointer) + return i; + } + + return -1; +} + +MemoryEntry * get_memory_entry(void * pointer) { + int position = find_memory_position(pointer); + + if (position == -1) + return NULL; + + return allocation_memory[position]; +} + +void move_memory_back(int position_to_start_on) { + for(int i = position_to_start_on ; i < number_of_memory_entries - 1 ; i++) { + allocation_memory[i] = allocation_memory[i+1]; + } +} \ No newline at end of file diff --git a/src/memory.h b/src/memory.h deleted file mode 100644 index 38ac590f..00000000 --- a/src/memory.h +++ /dev/null @@ -1,12 +0,0 @@ -#ifndef MEMORY_HEADER -#define MEMORY_HEADER - - -typedef struct MemoryPool_ MemoryPool; - -MemoryPool *create_memory_pool(); -void free_memory_pool(MemoryPool *pool); -void *memory_pool_allocate(MemoryPool *pool, size_t bytes); -void *memory_pool_reallocate(MemoryPool *pool, void *pointer, size_t bytes); - -#endif diff --git a/src/message_formatting.c b/src/message_formatting.c index 063e2b21..4d80df5a 100644 --- a/src/message_formatting.c +++ b/src/message_formatting.c @@ -18,6 +18,7 @@ #endif #include "constraint_internal.h" +#include // Handling of percent signs diff --git a/src/messaging.c b/src/messaging.c index 8f7360db..0778a8f8 100644 --- a/src/messaging.c +++ b/src/messaging.c @@ -19,6 +19,7 @@ #include #endif +#include #define message_content_size(Type) (sizeof(Type) - sizeof(long)) @@ -61,6 +62,7 @@ int get_pipe_write_handle(void) static void clean_up_messaging(void); +#undef realloc int start_cgreen_messaging(int tag) { CgreenMessageQueue *tmp; int pipes[2]; @@ -95,6 +97,7 @@ int start_cgreen_messaging(int tag) { queues[queue_count - 1].tag = tag; return queue_count - 1; } +#define realloc cgreen_realloc void send_cgreen_message(int messaging, int result) { CgreenMessage *message; @@ -128,6 +131,7 @@ int receive_cgreen_message(int messaging) { return result; } +#undef free static void clean_up_messaging(void) { int i; for (i = 0; i < queue_count; i++) { @@ -140,5 +144,6 @@ static void clean_up_messaging(void) { queues = NULL; queue_count = 0; } +#define free cgreen_free /* vim: set ts=4 sw=4 et cindent: */ diff --git a/src/posix_cgreen_pipe.c b/src/posix_cgreen_pipe.c index b61ac1a5..455a0087 100644 --- a/src/posix_cgreen_pipe.c +++ b/src/posix_cgreen_pipe.c @@ -18,7 +18,8 @@ /* One way to do it the old way is: ioctl(fd, FIOBIO, &flags); */ #endif - +#undef malloc +#undef free int cgreen_pipe_open(int pipes[2]) { int pipe_open_result; @@ -70,5 +71,6 @@ ssize_t cgreen_pipe_write(int p, const void *buf, size_t count) } return pipe_write_result; } - +#define malloc cgreen_malloc +#define free cgreen_free /* vim: set ts=4 sw=4 et cindent: */ diff --git a/src/utils.c b/src/utils.c index 2242eee5..b6ed1258 100644 --- a/src/utils.c +++ b/src/utils.c @@ -4,6 +4,7 @@ #include #include #include +#include char *string_dup(const char *string) { diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 5363c1b1..53aed05f 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -178,6 +178,11 @@ set(xml_output_library_SRCS xml_output_tests.c) add_library(${xml_output_library} SHARED ${xml_output_library_SRCS}) target_link_libraries(${xml_output_library} ${CGREEN_LIBRARY}) +set(memory_leak_library memory_leak_tests) +set(memory_leak_library_SRCS memory_leak_tests.c) +add_library(${memory_leak_library} SHARED ${memory_leak_library_SRCS}) +target_link_libraries(${memory_leak_library} ${CGREEN_LIBRARY}) + set(TEST_TARGET_LIBRARIES ${CGREEN_LIBRARY}) macro_add_test( @@ -224,5 +229,12 @@ macro_add_test(NAME xml_output ${xml_output_library}.expected ) +macro_add_test(NAME memory_leaks + COMMAND ${CMAKE_CURRENT_SOURCE_DIR}/../tools/cgreen_runner_output_diff + memory_leak_tests # Name + ${CMAKE_CURRENT_SOURCE_DIR} # Where sources are + ${memory_leak_library}.expected +) + # add verification that all public api is available as it should add_subdirectory(api) diff --git a/tests/memory_leak_tests.c b/tests/memory_leak_tests.c new file mode 100644 index 00000000..993fe720 --- /dev/null +++ b/tests/memory_leak_tests.c @@ -0,0 +1,91 @@ +#include +#include +#include +#include +#include "include/cgreen/memory.h" + +Describe(MemoryLeak) +BeforeEach(MemoryLeak) {} +AfterEach(MemoryLeak) {} + +Ensure(MemoryLeak, free_null_pointer) { + free(NULL); +} + +Ensure(MemoryLeak, alloc_and_free_memory) { + int* new_int = malloc(sizeof(int)); + free(new_int); + check_memory_leak(get_test_reporter()); + assert_that(true, is_true); +} + +Ensure(MemoryLeak, multiple_allocations_and_free_of_memory_in_different_order_works) { + int* new_int = malloc(sizeof(int)); + int* new_int1 = malloc(sizeof(int)); + int* new_int2 = malloc(sizeof(int)); + + free(new_int); + + int* new_int3 = malloc(sizeof(int)); + + free(new_int2); + free(new_int1); + free(new_int3); + + check_memory_leak(get_test_reporter()); + assert_that(true, is_true); +} + +Ensure(MemoryLeak, free_unallocate_memory) { + free((void*)1); + + check_memory_leak(get_test_reporter()); +} + +Ensure(MemoryLeak, fail_test_when_memory_is_leaked) { + malloc(sizeof(int)); + + check_memory_leak(get_test_reporter()); +} + +Ensure(MemoryLeak, use_realloc_maintain_data_in_string) { + char * new_pointer = malloc(sizeof(char) * 5); + strcpy(new_pointer, "123"); + + char* new_string = realloc(new_pointer, sizeof(char) * 10); + assert_that(new_string, is_equal_to_string("123")); + + free(new_string); +} + +Ensure(MemoryLeak, use_realloc_to_increase_the_size_of_a_pointer) { + char * new_pointer = malloc(sizeof(char) * 5); + + char* new_string = realloc(new_pointer, sizeof(char) * 10); + strcpy(new_string, "123456789"); + free(new_string); + check_memory_leak(get_test_reporter()); +} + +Ensure(MemoryLeak, use_realloc_to_decrease_the_size_of_a_pointer) { + char * new_pointer = malloc(sizeof(char) * 10); + + char* new_string = realloc(new_pointer, sizeof(char) * 5); + strcpy(new_string, "1234"); + + free(new_string); + check_memory_leak(get_test_reporter()); +} + +Ensure(MemoryLeak, use_calloc_to_allocate_an_array) { + int * array = calloc(5, sizeof(int)); + + array[0] = 1; + array[1] = 1; + array[2] = 1; + array[3] = 1; + array[4] = 1; + + free(array); + check_memory_leak(get_test_reporter()); +} diff --git a/tests/memory_leak_tests.expected b/tests/memory_leak_tests.expected new file mode 100644 index 00000000..2c5a26ad --- /dev/null +++ b/tests/memory_leak_tests.expected @@ -0,0 +1,12 @@ +Running "memory_leak_tests" (9 tests)... +memory_leak_tests.c: Failure: MemoryLeak -> fail_test_when_memory_is_leaked + Memory allocated [...] was not freed [memory_leak_tests.c:46] + +memory_leak_tests.c: Failure: MemoryLeak -> free_null_pointer + Trying to free memory using a NULL pointer + +memory_leak_tests.c: Failure: MemoryLeak -> free_unallocate_memory + Trying to free memory [0x1] that was not previously allocated + + "MemoryLeak": 3 passes, 1 failure, 2 exceptions in 0ms. +Completed "memory_leak_tests": 3 passes, 1 failure, 2 exceptions in 0ms. diff --git a/tests/normalize_memory_leak_tests.sed b/tests/normalize_memory_leak_tests.sed new file mode 100644 index 00000000..cedb64c0 --- /dev/null +++ b/tests/normalize_memory_leak_tests.sed @@ -0,0 +1,8 @@ +# 'time="?.?????"' => time="0.00000" +s/time=".+"/time="0.00000"/g +s/line=".+"/line="0"/g +# filenames, suites, libraries and "classnames" starting with "lib" or "cyg" +s/"lib/"/g +s/"cyg/"/g +# pointers address to ... +s/y allocated \[[0-9xa-f]+\]/y allocated \[...\]/g diff --git a/tools/cgreen-runner.c b/tools/cgreen-runner.c index d3a6ae96..6a30905d 100644 --- a/tools/cgreen-runner.c +++ b/tools/cgreen-runner.c @@ -13,7 +13,10 @@ #include "runner.h" #include "discoverer.h" - +#undef free +#undef malloc +#undef calloc +#undef realloc /*----------------------------------------------------------------------*/ static int file_exists(const char *filename) { @@ -66,8 +69,8 @@ static char* get_a_suite_name(const char *suite_option, const char *test_library char *suite_name; char *s; /* basename can return the parameter or an internal static string. Work around this. */ - s = string_dup(test_library_name); - suite_name = string_dup(basename(s)); + s = strdup(test_library_name); + suite_name = strdup(basename(s)); free(s); if ((s = strchr(suite_name, '.'))) { *s = '\0'; diff --git a/tools/runner.c b/tools/runner.c index 29795110..ad7c0ee2 100644 --- a/tools/runner.c +++ b/tools/runner.c @@ -30,6 +30,10 @@ typedef struct ContextSuite { struct ContextSuite *next; } ContextSuite; +#undef free +#undef malloc +#undef calloc +#undef realloc /*----------------------------------------------------------------------*/ static void destroy_context_suites(ContextSuite *context_suite) { if (context_suite != NULL) { @@ -48,10 +52,10 @@ static char *context_name_of(const char* symbolic_name) { char *context_name; if (strchr(symbolic_name, ':')) { - context_name = string_dup(symbolic_name); + context_name = strdup(symbolic_name); *strchr(context_name, ':') = '\0'; } else { - context_name = string_dup(CGREEN_DEFAULT_SUITE); + context_name = strdup(CGREEN_DEFAULT_SUITE); } return context_name; @@ -62,10 +66,10 @@ static char *context_name_of(const char* symbolic_name) { static char *test_name_of(const char *symbolic_name) { const char *colon = strchr(symbolic_name, ':'); if (colon) { - return string_dup(colon+1); + return strdup(colon+1); } - return string_dup(symbolic_name); + return strdup(symbolic_name); } @@ -105,7 +109,7 @@ static TestSuite *find_suite_for_context(ContextSuite *suites, const char *conte static ContextSuite *add_new_context_suite(TestSuite *parent, const char* context_name, ContextSuite *next) { ContextSuite *new_context_suite = (ContextSuite *)calloc(1, sizeof(ContextSuite)); - new_context_suite->context_name = string_dup(context_name); + new_context_suite->context_name = strdup(context_name); new_context_suite->suite = create_named_test_suite(context_name); new_context_suite->next = next; add_suite_(parent, context_name, new_context_suite->suite);