diff --git a/bindings/guile/gnc-optiondb.i b/bindings/guile/gnc-optiondb.i index 3340e24637..c2ac2e432a 100644 --- a/bindings/guile/gnc-optiondb.i +++ b/bindings/guile/gnc-optiondb.i @@ -1725,13 +1725,13 @@ gnc_register_multichoice_callback_option(GncOptionDBPtr& db, static void test_book_set_data(QofBook* book, const char* key, void* data) { - qof_book_set_data(book, key, data); + qof_book_set_data(book, key, data); } static void test_book_clear_data(QofBook* book, const char* key) { - qof_book_set_data(book, key, nullptr); + qof_book_set_data(book, key, nullptr); } static void diff --git a/libgnucash/backend/dbi/gnc-backend-dbi.cpp b/libgnucash/backend/dbi/gnc-backend-dbi.cpp index 1235ad1b9f..f4b83b4117 100644 --- a/libgnucash/backend/dbi/gnc-backend-dbi.cpp +++ b/libgnucash/backend/dbi/gnc-backend-dbi.cpp @@ -1030,7 +1030,7 @@ template<> bool QofDbiBackendProvider::type_check(const char *uri) { FILE* f; - gchar buf[50]; + gchar buf[51]{}; G_GNUC_UNUSED size_t chars_read; gint status; gchar* filename; @@ -1050,7 +1050,7 @@ QofDbiBackendProvider::type_check(const char *uri) } // OK if file has the correct header - chars_read = fread (buf, sizeof (buf), 1, f); + chars_read = fread (buf, sizeof (buf) - 1, 1, f); status = fclose (f); if (status < 0) { diff --git a/libgnucash/backend/xml/gnc-xml-backend.cpp b/libgnucash/backend/xml/gnc-xml-backend.cpp index 7894d02f57..6f48e574f1 100644 --- a/libgnucash/backend/xml/gnc-xml-backend.cpp +++ b/libgnucash/backend/xml/gnc-xml-backend.cpp @@ -234,7 +234,9 @@ GncXmlBackend::load(QofBook* book, QofBackendLoadType loadType) if (loadType != LOAD_TYPE_INITIAL_LOAD) return; error = ERR_BACKEND_NO_ERR; - m_book = book; + if (m_book) + g_object_unref(m_book); + m_book = QOF_BOOK(g_object_ref(book)); int rc; switch (determine_file_type (m_fullpath)) @@ -306,7 +308,8 @@ GncXmlBackend::sync(QofBook* book) * for multiple books have been removed in the meantime and there is just one * book, no more. */ - if (m_book == nullptr) m_book = book; + if (m_book == nullptr) + m_book = QOF_BOOK(g_object_ref(book)); if (book != m_book) return; if (qof_book_is_readonly (m_book)) diff --git a/libgnucash/engine/Split.c b/libgnucash/engine/Split.c index 252babe507..424ae16a9a 100644 --- a/libgnucash/engine/Split.c +++ b/libgnucash/engine/Split.c @@ -720,9 +720,14 @@ xaccFreeSplit (Split *split) split->date_reconciled = 0; G_OBJECT_CLASS (QOF_INSTANCE_GET_CLASS (&split->inst))->dispose(G_OBJECT (split)); - // Is this right? - if (split->gains_split) split->gains_split->gains_split = NULL; - /* qof_instance_release(&split->inst); */ + + if (split->gains_split) + { + Split *other = xaccSplitGetOtherSplit(split->gains_split); + split->gains_split->gains_split = NULL; + other->gains_split = NULL; + } + g_object_unref(split); } diff --git a/libgnucash/engine/gnc-date.cpp b/libgnucash/engine/gnc-date.cpp index 3dc541313d..e7f0d49aea 100644 --- a/libgnucash/engine/gnc-date.cpp +++ b/libgnucash/engine/gnc-date.cpp @@ -341,7 +341,8 @@ gnc_date_string_to_dateformat(const char* fmt_str, QofDateFormat *format) const char* gnc_date_monthformat_to_string(GNCDateMonthFormat format) { - switch (format) + //avoid UB if format is out of range + switch (static_cast(format)) { case GNCDATE_MONTH_NUMBER: return "number"; @@ -438,7 +439,9 @@ QofDateFormat qof_date_format_get (void) void qof_date_format_set(QofDateFormat df) { - if (df >= DATE_FORMAT_FIRST && df <= DATE_FORMAT_LAST) +//avoid UB if df is out of range + auto dfi{static_cast(df)}; + if (dfi >= DATE_FORMAT_FIRST && dfi <= DATE_FORMAT_LAST) { prevQofDateFormat = dateFormat; dateFormat = df; diff --git a/libgnucash/engine/gnc-timezone.cpp b/libgnucash/engine/gnc-timezone.cpp index b1db4769b4..5584e46e51 100644 --- a/libgnucash/engine/gnc-timezone.cpp +++ b/libgnucash/engine/gnc-timezone.cpp @@ -477,8 +477,8 @@ namespace IANAParser endian_swap(&info.gmtoff); tzinfo.push_back( {info, &fileblock[abbrev + info.abbrind], - fileblock[std_dist + index] != '\0', - fileblock[gmt_dist + index] != '\0'}); + (index < isstd_count ? fileblock[std_dist + index] != '\0' : true), + (index < isgmt_count ? fileblock[gmt_dist + index] != '\0' : false)}); } } diff --git a/libgnucash/engine/qofbook.cpp b/libgnucash/engine/qofbook.cpp index 13bdb71f5d..7575f7a72f 100644 --- a/libgnucash/engine/qofbook.cpp +++ b/libgnucash/engine/qofbook.cpp @@ -32,6 +32,7 @@ * Copyright (c) 2000 Dave Peticolas * Copyright (c) 2007 David Hampton */ +#include "qof-string-cache.h" #include #include @@ -53,6 +54,7 @@ #include "qofobject-p.h" #include "qofbookslots.h" #include "kvp-frame.hpp" +#include "gnc-lot.h" // For GNC_ID_ROOT_ACCOUNT: #include "AccountP.h" @@ -111,7 +113,8 @@ qof_book_init (QofBook *book) qof_instance_init_data (&book->inst, QOF_ID_BOOK, book); - book->data_tables = g_hash_table_new (g_str_hash, g_str_equal); + book->data_tables = g_hash_table_new_full (g_str_hash, g_str_equal, + (GDestroyNotify)qof_string_cache_remove, NULL); book->data_table_finalizers = g_hash_table_new (g_str_hash, g_str_equal); book->book_open = 'y'; @@ -317,6 +320,13 @@ qof_book_finalize_real (G_GNUC_UNUSED GObject *bookp) { } +static void +destroy_lot(QofInstance *inst, [[maybe_unused]]void* data) +{ + auto lot{GNC_LOT(inst)}; + gnc_lot_destroy(lot); +} + void qof_book_destroy (QofBook *book) { @@ -333,6 +343,11 @@ qof_book_destroy (QofBook *book) */ g_hash_table_foreach (book->data_table_finalizers, book_final, book); + /* Lots hold a variety of pointers that need to still exist while + * cleaning them up so run its book_end before the rest. + */ + auto lots{qof_book_get_collection(book, GNC_ID_LOT)}; + qof_collection_foreach(lots, destroy_lot, nullptr); qof_object_book_end (book); g_hash_table_destroy (book->data_table_finalizers); @@ -350,7 +365,6 @@ qof_book_destroy (QofBook *book) cols = book->hash_of_collections; g_object_unref (book); g_hash_table_destroy (cols); - /*book->hash_of_collections = NULL;*/ LEAVE ("book=%p", book); } @@ -450,13 +464,14 @@ qof_book_set_backend (QofBook *book, QofBackend *be) /* ====================================================================== */ /* Store arbitrary pointers in the QofBook for data storage extensibility */ -/* XXX if data is NULL, we should remove the key from the hash table! - */ void qof_book_set_data (QofBook *book, const char *key, gpointer data) { if (!book || !key) return; - g_hash_table_insert (book->data_tables, (gpointer)key, data); + if (data) + g_hash_table_insert (book->data_tables, (gpointer)CACHE_INSERT(key), data); + else + g_hash_table_remove(book->data_tables, key); } void diff --git a/libgnucash/engine/test/test-guid.cpp b/libgnucash/engine/test/test-guid.cpp index 2f14d2bc66..78a4e5c4f6 100644 --- a/libgnucash/engine/test/test-guid.cpp +++ b/libgnucash/engine/test/test-guid.cpp @@ -82,7 +82,7 @@ run_test (void) auto ent = QOF_INSTANCE(g_object_new(QOF_TYPE_INSTANCE, "guid", &guid, NULL)); do_test ((NULL == qof_collection_lookup_entity (col, &guid)), "duplicate guid"); - ent->e_type = type; + ent->e_type = CACHE_INSERT(type); qof_collection_insert_entity (col, ent); do_test ((NULL != qof_collection_lookup_entity (col, &guid)), "guid not found"); diff --git a/libgnucash/engine/test/test-qofbook.c b/libgnucash/engine/test/test-qofbook.c index f131bcbb4c..e74d37a7e7 100644 --- a/libgnucash/engine/test/test-qofbook.c +++ b/libgnucash/engine/test/test-qofbook.c @@ -962,10 +962,7 @@ test_book_new_destroy( void ) qof_book_set_data_fin( book, key, (gpointer) data, mock_final_cb ); test_struct.called = FALSE; - g_test_message( "Testing book destroy" ); qof_book_destroy( book ); - g_assert_true( qof_book_shutting_down( book ) ); - g_assert_true( test_struct.called ); } void diff --git a/libgnucash/engine/test/test-qofobject.c b/libgnucash/engine/test/test-qofobject.c index fae1379587..75d41c452a 100644 --- a/libgnucash/engine/test/test-qofobject.c +++ b/libgnucash/engine/test/test-qofobject.c @@ -303,8 +303,8 @@ test_qof_object_book_begin( Fixture *fixture, gconstpointer pData ) g_assert_cmpint( g_list_index( get_book_list(), (gconstpointer) book2 ), != , -1 ); g_assert_cmpint( object_book_begin_struct.call_count, == , list_length ); - qof_object_foreach_type ((QofForeachTypeCB)g_free, NULL); qof_book_destroy( book2 ); + qof_object_foreach_type ((QofForeachTypeCB)g_free, NULL); } static void @@ -389,8 +389,8 @@ test_qof_object_is_dirty( Fixture *fixture, gconstpointer pData ) g_assert_true( qof_object_is_dirty( book ) == TRUE ); g_assert_cmpint( object_dirty_struct.call_count, == , 1 ); /* should break on first */ - qof_object_foreach_type ((QofForeachTypeCB)g_free, NULL); qof_book_destroy( book ); + qof_object_foreach_type ((QofForeachTypeCB)g_free, NULL); } static struct @@ -433,8 +433,8 @@ test_qof_object_mark_clean( Fixture *fixture, gconstpointer pData ) qof_object_mark_clean( book ); g_assert_cmpint( object_mark_clean_struct.call_count, == , list_length ); - qof_object_foreach_type ((QofForeachTypeCB)g_free, NULL); qof_book_destroy( book ); + qof_object_foreach_type ((QofForeachTypeCB)g_free, NULL); } static struct