From 3abc9a5558b8ed2706fdf277fdc699cb2b6abae0 Mon Sep 17 00:00:00 2001 From: Richard Cohen Date: Tue, 7 Feb 2023 16:48:17 +0000 Subject: [PATCH] Refactor: Remove some unnecessary uses of memcpy - Also, remove unnecessary "static" in gnucash-style.c The second one in guid.cpp is UB: libgnucash/engine/guid.cpp:137:5: warning: undefined behavior, source object type 'const gnc::GUID' is not TriviallyCopyable [bugprone-undefined-memory-manipulation] memcpy (&target, &source, sizeof (GncGUID)); ^ --- CMakeLists.txt | 1 - common/config.h.cmake.in | 3 --- gnucash/gnome-utils/gnc-autoclear.c | 8 ++++---- gnucash/register/register-gnome/gnucash-style.c | 8 +++----- libgnucash/backend/sql/gnc-slots-sql.cpp | 2 +- libgnucash/backend/sql/gnc-sql-column-table-entry.cpp | 2 +- libgnucash/engine/SchedXaction.c | 4 ++-- libgnucash/engine/gncOwner.c | 3 +-- libgnucash/engine/guid.cpp | 5 +++-- libgnucash/engine/qofquery.cpp | 6 +++--- 10 files changed, 18 insertions(+), 24 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index ca473c1998..ca2cfa2e3a 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -731,7 +731,6 @@ if (UNIX OR MINGW) set (HAVE_GETTIMEOFDAY 1) set (HAVE_GUILE 1) set (HAVE_LIBM 1) -set (HAVE_MEMCPY 1) set (STDC_HEADERS 1) set (_ALL_SOURCE 1) set (_GNU_SOURCE 1) diff --git a/common/config.h.cmake.in b/common/config.h.cmake.in index c1dbec0866..77d555ccfa 100644 --- a/common/config.h.cmake.in +++ b/common/config.h.cmake.in @@ -132,9 +132,6 @@ /* Define to 1 if you have the header file. */ #cmakedefine HAVE_MCHECK_H 1 -/* Define to 1 if you have the `memcpy' function. */ -#cmakedefine HAVE_MEMCPY 1 - /* Define to 1 if you have the header file. */ #cmakedefine HAVE_MEMORY_H 1 diff --git a/gnucash/gnome-utils/gnc-autoclear.c b/gnucash/gnome-utils/gnc-autoclear.c index ccea7afaf6..34cb35b8ac 100644 --- a/gnucash/gnome-utils/gnc-autoclear.c +++ b/gnucash/gnome-utils/gnc-autoclear.c @@ -61,9 +61,9 @@ static void sack_foreach_func(gpointer key, gpointer value, gpointer user_data) sack_foreach_data_t data = (sack_foreach_data_t) user_data; gnc_numeric thisvalue = *(gnc_numeric *) key; gnc_numeric reachable_value = gnc_numeric_add_fixed (thisvalue, data->split_value); - gpointer new_value = g_malloc(sizeof(gnc_numeric)); + gnc_numeric* new_value = g_new(gnc_numeric, 1); - memcpy(new_value, &reachable_value, sizeof(gnc_numeric)); + *new_value = reachable_value; data->reachable_list = g_list_prepend(data->reachable_list, new_value); } @@ -108,7 +108,7 @@ gnc_account_get_autoclear_splits (Account *account, gnc_numeric toclear_value, { Split *split = (Split *)node->data; gnc_numeric split_value = xaccSplitGetAmount (split); - gpointer new_value = g_malloc(sizeof(gnc_numeric)); + gnc_numeric *new_value = g_new(gnc_numeric, 1); struct _sack_foreach_data_t s_data[1]; s_data->split_value = split_value; @@ -118,7 +118,7 @@ gnc_account_get_autoclear_splits (Account *account, gnc_numeric toclear_value, g_hash_table_foreach (sack, sack_foreach_func, s_data); /* Add the value of the split itself to the reachable_list */ - memcpy(new_value, &split_value, sizeof(gnc_numeric)); + *new_value = split_value; s_data->reachable_list = g_list_prepend (s_data->reachable_list, new_value); diff --git a/gnucash/register/register-gnome/gnucash-style.c b/gnucash/register/register-gnome/gnucash-style.c index 608fe44983..da8c604529 100644 --- a/gnucash/register/register-gnome/gnucash-style.c +++ b/gnucash/register/register-gnome/gnucash-style.c @@ -56,12 +56,10 @@ style_get_key (SheetBlockStyle *style) static gpointer style_create_key (SheetBlockStyle *style) { - static gint key; - gpointer new_key; + gint key = style->cursor->num_rows; - key = style->cursor->num_rows; - new_key = g_malloc(sizeof(key)); - new_key = memcpy(new_key, &key, sizeof(key)); + gint *new_key = g_new(gint, 1); + *new_key = key; return new_key; } diff --git a/libgnucash/backend/sql/gnc-slots-sql.cpp b/libgnucash/backend/sql/gnc-slots-sql.cpp index 0e9fc17a84..dfc43d179c 100644 --- a/libgnucash/backend/sql/gnc-slots-sql.cpp +++ b/libgnucash/backend/sql/gnc-slots-sql.cpp @@ -160,7 +160,7 @@ _retrieve_guid_ (gpointer pObject, gpointer pValue) g_return_if_fail (pObject != NULL); g_return_if_fail (pValue != NULL); - memcpy (pGuid, guid, sizeof (GncGUID)); + *pGuid = *guid; } /* Special column table because we need to be able to access the table by diff --git a/libgnucash/backend/sql/gnc-sql-column-table-entry.cpp b/libgnucash/backend/sql/gnc-sql-column-table-entry.cpp index cdd78fd13e..c1f829e1b0 100644 --- a/libgnucash/backend/sql/gnc-sql-column-table-entry.cpp +++ b/libgnucash/backend/sql/gnc-sql-column-table-entry.cpp @@ -649,7 +649,7 @@ _retrieve_guid_ (gpointer pObject, gpointer pValue) g_return_if_fail (pObject != NULL); g_return_if_fail (pValue != NULL); - memcpy (pGuid, guid, sizeof (GncGUID)); + *pGuid = *guid; } // Table to retrieve just the guid diff --git a/libgnucash/engine/SchedXaction.c b/libgnucash/engine/SchedXaction.c index 18d37ef6d8..5ad3480b9b 100644 --- a/libgnucash/engine/SchedXaction.c +++ b/libgnucash/engine/SchedXaction.c @@ -1118,8 +1118,8 @@ gnc_sx_clone_temporal_state (SXTmpStateData *tsd) if(tsd) { - toRet = g_malloc(sizeof(SXTmpStateData)); - toRet = memcpy (toRet, tsd, sizeof (SXTmpStateData)); + toRet = g_new(SXTmpStateData, 1); + *toRet = *tsd; } return toRet; diff --git a/libgnucash/engine/gncOwner.c b/libgnucash/engine/gncOwner.c index 5fcc0b24f7..0f44dfadc6 100644 --- a/libgnucash/engine/gncOwner.c +++ b/libgnucash/engine/gncOwner.c @@ -33,7 +33,6 @@ #include #include -#include /* for memcpy() */ #include #include "gncCustomerP.h" @@ -399,7 +398,7 @@ void gncOwnerCopy (const GncOwner *src, GncOwner *dest) { if (!src || !dest) return; if (src == dest) return; - memcpy (dest, src, sizeof (*dest)); + *dest = *src; } gboolean gncOwnerEqual (const GncOwner *a, const GncOwner *b) diff --git a/libgnucash/engine/guid.cpp b/libgnucash/engine/guid.cpp index d854b45a02..aaf58c53ad 100644 --- a/libgnucash/engine/guid.cpp +++ b/libgnucash/engine/guid.cpp @@ -56,6 +56,7 @@ #include #include #include +#include /* This static indicates the debugging module that this .o belongs to. */ static QofLogModule log_module = QOF_MOD_ENGINE; @@ -120,7 +121,7 @@ guid_copy (const GncGUID *guid) { if (!guid) return nullptr; auto ret = guid_malloc (); - memcpy (ret, guid, sizeof (GncGUID)); + *ret = *guid; return ret; } @@ -134,7 +135,7 @@ guid_null (void) static void guid_assign (GncGUID & target, gnc::GUID const & source) { - memcpy (&target, &source, sizeof (GncGUID)); + std::copy (source.begin(), source.end(), target.reserved); } /*Takes an allocated guid pointer and constructs it in place*/ diff --git a/libgnucash/engine/qofquery.cpp b/libgnucash/engine/qofquery.cpp index dacf105e05..f805fbed55 100644 --- a/libgnucash/engine/qofquery.cpp +++ b/libgnucash/engine/qofquery.cpp @@ -188,7 +188,7 @@ static QofQueryTerm * copy_query_term (const QofQueryTerm *qt) if (!qt) return NULL; new_qt = g_new0 (QofQueryTerm, 1); - memcpy (new_qt, qt, sizeof(QofQueryTerm)); + *new_qt = *qt; new_qt->param_list = g_slist_copy (qt->param_list); new_qt->param_fcns = g_slist_copy (qt->param_fcns); new_qt->pdata = qof_query_core_predicate_copy (qt->pdata); @@ -224,7 +224,7 @@ copy_or_terms(const GList * or_terms) static void copy_sort (QofQuerySort *dst, const QofQuerySort *src) { - memcpy (dst, src, sizeof (*dst)); + *dst = *src; dst->param_list = g_slist_copy (src->param_list); dst->param_fcns = g_slist_copy (src->param_fcns); } @@ -1006,7 +1006,7 @@ QofQuery * qof_query_copy (QofQuery *q) ht = copy->be_compiled; free_members (copy); - memcpy (copy, q, sizeof (QofQuery)); + *copy = *q; copy->be_compiled = ht; copy->terms = copy_or_terms (q->terms);