From 82f783595f654b62ce57a7cfb537d23efce7affc Mon Sep 17 00:00:00 2001 From: Aldrik Ramaekers Date: Sun, 19 Oct 2025 22:00:50 +0200 Subject: fix r/w mem leaks --- TODO | 1 - include/administration.hpp | 2 ++ libs/xml.c/src/xml.c | 10 +++++++++- libs/xml.c/src/xml.h | 2 +- run.bat | 1 + src/administration.cpp | 24 ++++++++++++++++++---- src/administration_reader.cpp | 46 ++++++++++++++++++++++++++++++++++++------- src/administration_writer.cpp | 13 ++++++------ src/main.cpp | 1 + src/ui/ui_expenses.cpp | 1 + src/ui/ui_invoices.cpp | 1 + 11 files changed, 82 insertions(+), 20 deletions(-) diff --git a/TODO b/TODO index 4cc4806..57a1d99 100644 --- a/TODO +++ b/TODO @@ -1,7 +1,6 @@ TODO: Refactor: -- refactor _add functions to use _import functions - There is alot of memory leakage Testing: diff --git a/include/administration.hpp b/include/administration.hpp index 6a284f9..5680418 100644 --- a/include/administration.hpp +++ b/include/administration.hpp @@ -451,6 +451,7 @@ namespace administration { void create_empty(char* save_file); void create_default(char* save_file); void create_from_file(char* save_file); + void destroy(); // Callback functions. // ======================= @@ -564,6 +565,7 @@ namespace administration { a_err invoice_remove(invoice* invoice); void invoice_set_currency(invoice* invoice, char* currency); invoice invoice_create_copy(invoice* invoice); + void invoice_destroy(invoice* invoice); void invoice_recalculate_totals(); a_err invoice_is_valid(invoice* invoice); diff --git a/libs/xml.c/src/xml.c b/libs/xml.c/src/xml.c index 0a93e57..6d14213 100644 --- a/libs/xml.c/src/xml.c +++ b/libs/xml.c/src/xml.c @@ -820,7 +820,15 @@ exit_failure: } - +bool xml_string_is_valid(uint8_t* buffer, size_t length) +{ + struct xml_document* doc = xml_parse_document(buffer, length); + if (!doc) { + return 0; + } + xml_document_free(doc, false); + return 1; +} /** diff --git a/libs/xml.c/src/xml.h b/libs/xml.c/src/xml.h index 65a7b9f..ef8d55c 100644 --- a/libs/xml.c/src/xml.h +++ b/libs/xml.c/src/xml.h @@ -64,7 +64,7 @@ struct xml_string; * @return The parsed xml fragment iff parsing was successful, 0 otherwise */ struct xml_document* xml_parse_document(uint8_t* buffer, size_t length); - +bool xml_string_is_valid(uint8_t* buffer, size_t length); /** diff --git a/run.bat b/run.bat index d097601..5ba0deb 100644 --- a/run.bat +++ b/run.bat @@ -51,3 +51,4 @@ cl %FLAGS% %INCLUDE_DIRS% %DEFINITIONS% %SOURCES% %LIB_SOURCES% /Fe%OUT_DIR%/%OU if "%1"=="-r" call "%OUT_DIR%/%OUT_EXE%.exe" C:\Users\aldri\Desktop\Vault\Projects\accounting\build\example.openbook if "%1"=="-t" call "%OUT_DIR%/%OUT_EXE%.exe" -v if "%1"=="-d" call devenv "%OUT_DIR%/%OUT_EXE%.exe" +if "%1"=="-l" call drmemory.exe -leaks_only -brief -batch -- "%OUT_DIR%/%OUT_EXE%.exe" C:\Users\aldri\Desktop\Vault\Projects\accounting\build\example.openbook \ No newline at end of file diff --git a/src/administration.cpp b/src/administration.cpp index 8af6e86..b3d5ad4 100644 --- a/src/administration.cpp +++ b/src/administration.cpp @@ -149,9 +149,20 @@ static void administration_destroy_list(list_t *list) list_destroy(list); } -void administration_destroy() +static void administration_destroy_invoices() +{ + list_iterator_start(&g_administration.invoices); + while (list_iterator_hasnext(&g_administration.invoices)) { + invoice* c = (invoice *)list_iterator_next(&g_administration.invoices); + administration_destroy_list(&c->billing_items); + } + list_iterator_stop(&g_administration.invoices); +} + +void administration::destroy() { is_initialized = false; + administration_destroy_invoices(); administration_destroy_list(&g_administration.invoices); administration_destroy_list(&g_administration.contacts); administration_destroy_list(&g_administration.projects); @@ -161,7 +172,7 @@ void administration_destroy() void administration::create_from_file(char* save_file) { - if (is_initialized) administration_destroy(); + if (is_initialized) administration::destroy(); administration_create(); strops::copy(g_administration.path, save_file, sizeof(g_administration.path)); @@ -170,7 +181,7 @@ void administration::create_from_file(char* save_file) void administration::create_empty(char* save_file) { - if (is_initialized) administration_destroy(); + if (is_initialized) administration::destroy(); administration_create(); g_administration.next_id = 2; @@ -1337,6 +1348,11 @@ bool administration::invoice_has_intra_community_services(invoice* invoice) return false; } +void administration::invoice_destroy(invoice* invoice) +{ + administration_destroy_list(&invoice->billing_items); +} + invoice administration::invoice_create_empty() { invoice result; @@ -1348,7 +1364,7 @@ invoice administration::invoice_create_empty() result.delivered_at = result.issued_at; result.expires_at = result.issued_at + administration::get_default_invoice_expire_duration(); - list_init(&result.billing_items); // @leak + list_init(&result.billing_items); strops::copy(result.currency, get_default_currency_for_country(g_administration.company_info.address.country_code), MAX_LEN_CURRENCY); return result; } diff --git a/src/administration_reader.cpp b/src/administration_reader.cpp index fdbe8b6..91325ea 100644 --- a/src/administration_reader.cpp +++ b/src/administration_reader.cpp @@ -75,13 +75,15 @@ bool administration_reader::open_existing(char* file_path) const char *name = zip_entry_name(zip); int isdir = zip_entry_isdir(zip); if (isdir) continue; - unsigned long long size = zip_entry_size(zip); - - char* buffer = (char*)memops::alloc(size+1); - memops::zero(buffer, size+1); + + char* buffer; + unsigned long long size; zip_entry_read(zip, (void**)&buffer, (size_t*)&size); - if (strops::empty(name)) continue; + if (strops::empty(name)) { + memops::unalloc(buffer); + continue; + } if (strops::equals(name, ADMIN_FILE_INFO)) { @@ -107,7 +109,6 @@ bool administration_reader::open_existing(char* file_path) { administration_reader::import_invoice(buffer, (size_t)size); } - memops::unalloc(buffer); } zip_entry_close(zip); @@ -129,7 +130,10 @@ bool administration_reader::read_invoice_from_xml(invoice* result, char* buffer, if (!document) return false; struct xml_node* root = xml_document_root(document); - if (!root) return false; + if (!root) { + xml_document_free(document, false); + return false; + } invoice data = administration::invoice_create_empty(); xml_get_str(root, data.id, MAX_LEN_ID, "cbc:ID"); @@ -238,6 +242,8 @@ bool administration_reader::read_invoice_from_xml(invoice* result, char* buffer, memops::unalloc(child_name); } + xml_document_free(document, false); + *result = data; return true; } @@ -257,6 +263,7 @@ bool administration_reader::import_invoice(char* buffer, size_t buffer_size) logger::aerr(result); logger::error("ERROR loading invoice '%s'.", data.sequential_number); } + administration::invoice_destroy(&data); return result == A_ERR_SUCCESS; } @@ -269,6 +276,10 @@ bool administration_reader::import_contact(char* buffer, size_t buffer_size) if (!document) return false; struct xml_node* root = xml_document_root(document); + if (!root) { + xml_document_free(document, false); + return false; + } contact data = {0}; xml_get_str(root, data.id, MAX_LEN_ID, "Id"); @@ -297,6 +308,7 @@ bool administration_reader::import_contact(char* buffer, size_t buffer_size) logger::error("ERROR loading contact '%s'.", data.name); } + xml_document_free(document, false); return result; } @@ -308,6 +320,10 @@ bool administration_reader::import_project(char* buffer, size_t buffer_size) if (!document) return false; struct xml_node* root = xml_document_root(document); + if (!root) { + xml_document_free(document, false); + return false; + } project data = {0}; xml_get_str(root, data.id, MAX_LEN_ID, "Id"); @@ -326,6 +342,7 @@ bool administration_reader::import_project(char* buffer, size_t buffer_size) logger::error("ERROR loading project '%s'.", data.id); } + xml_document_free(document, false); return result; } @@ -337,6 +354,10 @@ bool administration_reader::import_cost_center(char* buffer, size_t buffer_size) if (!document) return false; struct xml_node* root = xml_document_root(document); + if (!root) { + xml_document_free(document, false); + return false; + } cost_center data = {0}; xml_get_str(root, data.id, MAX_LEN_ID, "Id"); @@ -353,6 +374,7 @@ bool administration_reader::import_cost_center(char* buffer, size_t buffer_size) logger::error("ERROR loading cost center '%s'.", data.id); } + xml_document_free(document, false); return result; } @@ -364,6 +386,10 @@ bool administration_reader::import_tax_rate(char* buffer, size_t buffer_size) if (!document) return false; struct xml_node* root = xml_document_root(document); + if (!root) { + xml_document_free(document, false); + return false; + } tax_rate data = {0}; xml_get_str(root, data.internal_code, MAX_LEN_SHORT_DESC, "Id"); @@ -390,6 +416,7 @@ bool administration_reader::import_tax_rate(char* buffer, size_t buffer_size) logger::error("ERROR loading tax rate '%s'.", data.internal_code); } + xml_document_free(document, false); return result; } @@ -401,6 +428,10 @@ bool administration_reader::import_administration_info(char* buffer, size_t buff if (!document) return false; struct xml_node* root = xml_document_root(document); + if (!root) { + xml_document_free(document, false); + return false; + } administration::set_next_id(xml_get_s32(root, "NextId")); administration::set_next_sequence_number(xml_get_s32(root, "NextSequenceNumber")); @@ -414,5 +445,6 @@ bool administration_reader::import_administration_info(char* buffer, size_t buff logger::info("Loaded administration info in %.3fms. next_id=%d next_sequence_number=%d", STOPWATCH_TIME, administration::get_next_id(), administration::get_next_sequence_number()); + xml_document_free(document, false); return true; } \ No newline at end of file diff --git a/src/administration_writer.cpp b/src/administration_writer.cpp index b6428f4..04068e7 100644 --- a/src/administration_writer.cpp +++ b/src/administration_writer.cpp @@ -103,6 +103,7 @@ static bool zip_entry_exists(char* entry) { struct zip_t *zip_read = zip_open(administration::get_file_path(), 0, 'r'); int result = zip_entry_open(zip_read, entry); + zip_entry_close(zip_read); zip_close(zip_read); return result == 0; @@ -519,7 +520,7 @@ bool administration_writer::save_invoice_blocking(invoice inv) strops::format(final_path, 50, "%s.xml", inv.id); int final_length = (int)strops::length(file_content); - if (!xml_parse_document((uint8_t*)file_content, final_length)) result = 0; + if (!xml_string_is_valid((uint8_t*)file_content, final_length)) result = 0; else if (!write_to_zip(final_path, file_content, final_length)) result = 0; memops::unalloc(file_content); @@ -578,7 +579,7 @@ bool administration_writer::save_project_blocking(project project) strops::format(final_path, 50, "%s.xml", project.id); int final_length = (int)strops::length(file_content); - if (!xml_parse_document((uint8_t*)file_content, final_length)) result = 0; + if (!xml_string_is_valid((uint8_t*)file_content, final_length)) result = 0; else if (!write_to_zip(final_path, file_content, final_length)) result = 0; memops::unalloc(file_content); @@ -626,7 +627,7 @@ bool administration_writer::save_cost_center_blocking(cost_center cost) strops::format(final_path, 50, "%s.xml", cost.id); int final_length = (int)strops::length(file_content); - if (!xml_parse_document((uint8_t*)file_content, final_length)) result = 0; + if (!xml_string_is_valid((uint8_t*)file_content, final_length)) result = 0; else if (!write_to_zip(final_path, file_content, final_length)) result = 0; memops::unalloc(file_content); @@ -683,7 +684,7 @@ bool administration_writer::save_tax_rate_blocking(tax_rate rate) strops::format(final_path, 50, "T/%s.xml", rate.internal_code); int final_length = (int)strops::length(file_content); - if (!xml_parse_document((uint8_t*)file_content, final_length)) result = 0; + if (!xml_string_is_valid((uint8_t*)file_content, final_length)) result = 0; else if (!write_to_zip(final_path, file_content, final_length)) result = 0; memops::unalloc(file_content); @@ -746,7 +747,7 @@ bool administration_writer::save_contact_blocking(contact c) strops::format(final_path, 50, "%s.xml", c.id); int final_length = (int)strops::length(file_content); - if (!xml_parse_document((uint8_t*)file_content, final_length)) result = 0; + if (!xml_string_is_valid((uint8_t*)file_content, final_length)) result = 0; else if (!write_to_zip(final_path, file_content, final_length)) result = 0; memops::unalloc(file_content); @@ -802,7 +803,7 @@ bool administration_writer::save_all_administration_info_blocking() //// Write to Disk. int final_length = (int)strops::length(file_content); - if (!xml_parse_document((uint8_t*)file_content, final_length)) result = 0; + if (!xml_string_is_valid((uint8_t*)file_content, final_length)) result = 0; else if (!write_to_zip(ADMIN_FILE_INFO, file_content, final_length)) result = 0; memops::unalloc(file_content); diff --git a/src/main.cpp b/src/main.cpp index e0935f5..948678c 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -204,6 +204,7 @@ int main(int argc, char** argv) administration_writer::destroy(); timer_lib_shutdown(); + administration::destroy(); // Cleanup ImGui_ImplDX11_Shutdown(); diff --git a/src/ui/ui_expenses.cpp b/src/ui/ui_expenses.cpp index 6aa6e96..560eb90 100644 --- a/src/ui/ui_expenses.cpp +++ b/src/ui/ui_expenses.cpp @@ -40,6 +40,7 @@ void draw_invoice_items_form(invoice* invoice, bool outgoing = true); void ui::destroy_expenses() { + administration::invoice_destroy(&active_invoice); memops::unalloc(invoice_items_buffer); } diff --git a/src/ui/ui_invoices.cpp b/src/ui/ui_invoices.cpp index 277ce46..18acc59 100644 --- a/src/ui/ui_invoices.cpp +++ b/src/ui/ui_invoices.cpp @@ -37,6 +37,7 @@ void draw_addressee_form_ex(delivery_info* buffer, bool viewing_only = false); void ui::destroy_invoices() { + administration::invoice_destroy(&active_invoice); memops::unalloc(invoice_items_buffer); } -- cgit v1.2.3-70-g09d2