Fix a bunch of UB errors from ASAN about mismatched function types.

The casts fool the compiler but not the UB sanitizer.
This commit is contained in:
John Ralls 2024-02-20 15:33:11 -08:00
parent 7bd97f15d0
commit 226bfea108
8 changed files with 30 additions and 51 deletions

View File

@ -24,6 +24,7 @@
* *
\********************************************************************/
#include "qofinstance.h"
#include <config.h>
#include <platform.h>
@ -692,6 +693,11 @@ xaccSplitDump (const Split *split, const char *tag)
/********************************************************************\
\********************************************************************/
static void
do_destroy (QofInstance *inst)
{
xaccFreeSplit (GNC_SPLIT (inst));
}
void
xaccFreeSplit (Split *split)
@ -1044,8 +1050,7 @@ xaccSplitCommitEdit(Split *s)
original and new transactions, for the _next_ begin/commit cycle. */
s->orig_acc = s->acc;
s->orig_parent = s->parent;
if (!qof_commit_edit_part2(QOF_INSTANCE(s), commit_err, NULL,
(void (*) (QofInstance *)) xaccFreeSplit))
if (!qof_commit_edit_part2(QOF_INSTANCE(s), commit_err, NULL, do_destroy))
return;
if (acc)

View File

@ -24,6 +24,7 @@
* *
\********************************************************************/
#include "qofinstance.h"
#include <config.h>
#include <platform.h>
@ -1485,8 +1486,9 @@ destroy_gains (Transaction *trans)
}
static void
do_destroy (Transaction *trans)
do_destroy (QofInstance* inst)
{
Transaction *trans{GNC_TRANSACTION (inst)};
SplitList *node;
gboolean shutting_down = qof_book_shutting_down(qof_instance_get_book(trans));
@ -1551,8 +1553,10 @@ static gboolean was_trans_emptied(Transaction *trans)
return TRUE;
}
static void trans_on_error(Transaction *trans, QofBackendError errcode)
static void trans_on_error(QofInstance *inst, QofBackendError errcode)
{
Transaction *trans{GNC_TRANSACTION(inst)};
/* If the backend puked, then we must roll-back
* at this point, and let the user know that we failed.
* The GUI should check for error conditions ...
@ -1568,8 +1572,9 @@ static void trans_on_error(Transaction *trans, QofBackendError errcode)
gnc_engine_signal_commit_error( errcode );
}
static void trans_cleanup_commit(Transaction *trans)
static void trans_cleanup_commit(QofInstance *inst)
{
Transaction *trans{GNC_TRANSACTION(inst)};
GList *slist, *node;
/* ------------------------------------------------- */
@ -1684,11 +1689,8 @@ xaccTransCommitEdit (Transaction *trans)
}
trans->txn_type = TXN_TYPE_UNCACHED;
qof_commit_edit_part2(QOF_INSTANCE(trans),
(void (*) (QofInstance *, QofBackendError))
trans_on_error,
(void (*) (QofInstance *)) trans_cleanup_commit,
(void (*) (QofInstance *)) do_destroy);
qof_commit_edit_part2(QOF_INSTANCE(trans), trans_on_error,
trans_cleanup_commit, do_destroy);
LEAVE ("(trans=%p)", trans);
}
@ -1829,7 +1831,7 @@ xaccTransRollbackEdit (Transaction *trans)
* out about it until this user tried to edit it.
*/
xaccTransDestroy (trans);
do_destroy (trans);
do_destroy (QOF_INSTANCE(trans));
/* push error back onto the stack */
qof_backend_set_error (be, errcode);

View File

@ -184,10 +184,10 @@ typedef struct
void (*gen_event_trans)(Transaction*);
void (*xaccFreeTransaction)(Transaction*);
void (*destroy_gains)(Transaction*);
void (*do_destroy)(Transaction*);
void (*do_destroy)(QofInstance*);
gboolean (*was_trans_emptied)(Transaction*);
void (*trans_on_error)(Transaction*, QofBackendError);
void (*trans_cleanup_commit)(Transaction*);
void (*trans_on_error)(QofInstance*, QofBackendError);
void (*trans_cleanup_commit)(QofInstance*);
void (*xaccTransScrubGainsDate)(Transaction*);
Transaction *(*dupe_trans)(const Transaction*);

View File

@ -656,7 +656,7 @@ gnc_lot_remove_split (GNCLot *lot, Split *split)
xaccSplitSetLot(split, NULL);
priv->is_closed = LOT_CLOSED_UNKNOWN; /* force an is-closed computation */
if (NULL == priv->splits)
if (!priv->splits && priv->account)
{
xaccAccountRemoveLot (priv->account, lot);
priv->account = NULL;

View File

@ -858,29 +858,18 @@ test_xaccFreeAccount (Fixture *fixture, gconstpointer pData)
{
auto msg1 = "[xaccFreeAccount()] instead of calling xaccFreeAccount(), please call\n"
" xaccAccountBeginEdit(); xaccAccountDestroy();\n";
#ifdef USE_CLANG_FUNC_SIG
#define _func "int xaccTransGetSplitIndex(const Transaction *, const Split *)"
#else
#define _func "int xaccTransGetSplitIndex(const Transaction*, const Split*)"
#endif
auto msg2 = _func ": assertion 'trans && split' failed";
#undef _func
auto loglevel = static_cast<GLogLevelFlags>(G_LOG_LEVEL_CRITICAL | G_LOG_FLAG_FATAL);
auto check1 = test_error_struct_new("gnc.account", loglevel, msg1);
auto check2 = test_error_struct_new("gnc.engine", loglevel, msg2);
QofBook *book = gnc_account_get_book (fixture->acct);
Account *parent = gnc_account_get_parent (fixture->acct);
AccountPrivate *p_priv = fixture->func->get_private (parent);
const guint numItems = 3;
guint i = 0;
guint hdlr1, hdlr2;
guint hdlr1;
gnc_commodity *commodity = gnc_commodity_new (book, "US Dollar", "CURRENCY", "USD", "0", 100);
test_add_error (check1);
test_add_error (check2);
hdlr1 = g_log_set_handler ("gnc.account", loglevel,
(GLogFunc)test_checked_handler, check1);
hdlr2 = g_log_set_handler ("gnc.engine", loglevel,
(GLogFunc)test_checked_handler, check2);
g_test_log_set_fatal_handler ((GTestLogFatalFunc)test_list_handler, NULL);
for (i = 0; i < numItems; i++)
{
@ -897,7 +886,6 @@ test_xaccFreeAccount (Fixture *fixture, gconstpointer pData)
g_assert_true (p_priv->parent != NULL);
g_assert_true (p_priv->commodity != NULL);
g_assert_cmpint (check1->hits, ==, 0);
g_assert_cmpint (check2->hits, ==, 0);
/* Now set the other private parts to something so that they can be set back */
p_priv->cleared_balance = gnc_numeric_create ( 5, 12);
p_priv->reconciled_balance = gnc_numeric_create ( 5, 12);
@ -906,10 +894,8 @@ test_xaccFreeAccount (Fixture *fixture, gconstpointer pData)
p_priv->sort_dirty = TRUE;
fixture->func->xaccFreeAccount (parent);
g_assert_cmpint (check1->hits, ==, 6);
g_assert_cmpint (check2->hits, ==, 6);
/* cleanup what's left */
g_log_remove_handler ("gnc.account", hdlr1);
g_log_remove_handler ("gnc.engine", hdlr2);
test_clear_error_list ();
qof_book_destroy (book);
g_free (fixture->func);
@ -972,17 +958,9 @@ test_xaccAccountCommitEdit (Fixture *fixture, gconstpointer pData)
{
auto msg1 = "[xaccFreeAccount()] instead of calling xaccFreeAccount(), please call\n"
" xaccAccountBeginEdit(); xaccAccountDestroy();\n";
#ifdef USE_CLANG_FUNC_SIG
#define _func "int xaccTransGetSplitIndex(const Transaction *, const Split *)"
#else
#define _func "int xaccTransGetSplitIndex(const Transaction*, const Split*)"
#endif
auto msg2 = _func ": assertion 'trans && split' failed";
#undef _func
auto loglevel = static_cast<GLogLevelFlags>(G_LOG_LEVEL_CRITICAL | G_LOG_FLAG_FATAL);
auto check1 = test_error_struct_new("gnc.account", loglevel, msg1);
auto check2 = test_error_struct_new("gnc.engine", loglevel, msg2);
guint hdlr1, hdlr2;
guint hdlr1;
TestSignal sig1, sig2;
QofBook *book = gnc_account_get_book (fixture->acct);
Account *parent = gnc_account_get_parent (fixture->acct);
@ -991,11 +969,8 @@ test_xaccAccountCommitEdit (Fixture *fixture, gconstpointer pData)
guint i = 0;
gnc_commodity *commodity = gnc_commodity_new (book, "US Dollar", "CURRENCY", "USD", "0", 100);
test_add_error (check1);
test_add_error (check2);
hdlr1 = g_log_set_handler ("gnc.account", loglevel,
(GLogFunc)test_checked_handler, check1);
hdlr2 = g_log_set_handler ("gnc.engine", loglevel,
(GLogFunc)test_checked_handler, check2);
g_test_log_set_fatal_handler ((GTestLogFatalFunc)test_list_handler, NULL);
for (i = 0; i < numItems; i++)
{
@ -1012,7 +987,6 @@ test_xaccAccountCommitEdit (Fixture *fixture, gconstpointer pData)
g_assert_true (p_priv->parent != NULL);
g_assert_true (p_priv->commodity != NULL);
g_assert_cmpint (check1->hits, ==, 0);
g_assert_cmpint (check2->hits, ==, 0);
sig1 = test_signal_new (&parent->inst, QOF_EVENT_MODIFY, NULL);
sig2 = test_signal_new (&parent->inst, QOF_EVENT_DESTROY, NULL);
@ -1028,7 +1002,6 @@ test_xaccAccountCommitEdit (Fixture *fixture, gconstpointer pData)
g_assert_true (p_priv->parent != NULL);
g_assert_true (p_priv->commodity != NULL);
g_assert_cmpint (check1->hits, ==, 0);
g_assert_cmpint (check2->hits, ==, 0);
/* xaccAccountDestroy destroys the account by calling
* qof_instance_set_destroying (), then xaccAccountCommitEdit ();
*/
@ -1038,12 +1011,10 @@ test_xaccAccountCommitEdit (Fixture *fixture, gconstpointer pData)
test_signal_assert_hits (sig1, 2);
test_signal_assert_hits (sig2, 1);
g_assert_cmpint (check1->hits, ==, 2);
g_assert_cmpint (check2->hits, ==, 12);
/* And clean up */
test_signal_free (sig1);
test_signal_free (sig2);
g_log_remove_handler ("gnc.account", hdlr1);
g_log_remove_handler ("gnc.engine", hdlr2);
test_clear_error_list ();
qof_book_destroy (book);
g_free (fixture->func);

View File

@ -75,7 +75,7 @@ setup (Fixture *fixture, gconstpointer pData)
xaccTransSetCurrency (txn, fixture->curr);
xaccSplitSetParent (fixture->split, txn);
xaccTransCommitEdit (txn);
gnc_lot_set_account (lot, acc);
xaccAccountInsertLot (acc, lot);
fixture->split->action = CACHE_INSERT ("foo");
fixture->split->memo = CACHE_INSERT ("bar");
fixture->split->acc = acc;

View File

@ -1419,7 +1419,7 @@ test_do_destroy (GainsFixture *fixture, gconstpointer pData)
/* Protect against recursive calls to do_destroy from xaccTransCommitEdit */
xaccTransBeginEdit(base->txn);
base->func->do_destroy (base->txn);
base->func->do_destroy (QOF_INSTANCE(base->txn));
g_assert_cmpint (test_signal_return_hits (sig), ==, 1);
g_assert_true (base->txn->description == NULL);
g_assert_cmpint (GPOINTER_TO_INT(base->txn->num), ==, 1);
@ -1472,7 +1472,7 @@ test_trans_on_error (Fixture *fixture, gconstpointer pData)
gnc_engine_add_commit_error_callback ((EngineCommitErrorCallback)commit_error_cb, NULL);
xaccTransBeginEdit (fixture->txn);
g_assert_cmpint (qof_instance_get_editlevel (fixture->txn), ==, 1);
fixture->func->trans_on_error (fixture->txn, errcode);
fixture->func->trans_on_error (QOF_INSTANCE(fixture->txn), errcode);
g_assert_cmpint (check->hits, ==, 1);
g_assert_cmpint ((guint)errorvalue, ==, (guint)errcode);
g_assert_cmpint (qof_instance_get_editlevel (fixture->txn), ==, 0);
@ -1515,7 +1515,7 @@ test_trans_cleanup_commit (Fixture *fixture, gconstpointer pData)
/*Reverse the splits list so we can check later that it got sorted */
fixture->txn->splits = g_list_reverse (fixture->txn->splits);
g_assert_true (fixture->txn->splits->data != split0);
fixture->func->trans_cleanup_commit (fixture->txn);
fixture->func->trans_cleanup_commit (QOF_INSTANCE(fixture->txn));
g_assert_cmpint (test_signal_return_hits (sig_d_remove), ==, 1);
g_assert_cmpint (test_signal_return_hits (sig_b_remove), ==, 1);
@ -1540,7 +1540,7 @@ test_trans_cleanup_commit (Fixture *fixture, gconstpointer pData)
fixture->txn->orig = orig;
orig->num = fixture->txn->num;
g_object_ref (orig);
fixture->func->trans_cleanup_commit (fixture->txn);
fixture->func->trans_cleanup_commit (QOF_INSTANCE(fixture->txn));
g_assert_cmpint (test_signal_return_hits (sig_d_remove), ==, 2);
g_assert_cmpint (test_signal_return_hits (sig_b_remove), ==, 1);

View File

@ -656,6 +656,7 @@ libgnucash/engine/gnc-option-impl.cpp
libgnucash/engine/gncOrder.c
libgnucash/engine/gncOwner.c
libgnucash/engine/gnc-pricedb.cpp
libgnucash/engine/gnc-quote-source.cpp
libgnucash/engine/gnc-rational.cpp
libgnucash/engine/gnc-session.c
libgnucash/engine/gncTaxTable.c