Skip to content

Commit

Permalink
Bug 799423 - Crash when creating ETF transaction
Browse files Browse the repository at this point in the history 
Don't keep the old split if it's a trading split. The balance code
regenerates those, invalidating the pointer.
  • Loading branch information
@jralls
jralls committed Sep 30, 2024
1 parent 711554e commit 5c5b627
Showing 1 changed file with 14 additions and 5 deletions.
19 changes: 14 additions & 5 deletions gnucash/register/ledger-core/split-register-control.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -364,6 +364,12 @@ gnc_split_register_check_account (SplitRegister *reg,
return TRUE;
}

static inline bool
is_trading_split (Split* split)
{
return xaccAccountGetType (xaccSplitGetAccount (split)) == ACCT_TYPE_TRADING;
}

static void
gnc_split_register_move_cursor (VirtualLocation *p_new_virt_loc,
gpointer user_data)
Expand All @@ -374,10 +380,10 @@ gnc_split_register_move_cursor (VirtualLocation *p_new_virt_loc,
Transaction *pending_trans;
Transaction *new_trans;
Transaction *old_trans;
Split *old_trans_split;
Split *old_trans_split{nullptr};
Split *new_trans_split;
Split *new_split;
Split *old_split;
Split *old_split{nullptr};
CursorClass new_class;
CursorClass old_class;
gboolean exact_traversal;
Expand All @@ -399,10 +405,13 @@ gnc_split_register_move_cursor (VirtualLocation *p_new_virt_loc,
info = gnc_split_register_get_info (reg);

/* The transaction we are coming from */
old_split = gnc_split_register_get_current_split (reg);
if (auto s{gnc_split_register_get_current_split (reg)}; !is_trading_split(s))
old_split = s;
old_trans = gnc_split_register_get_current_trans (reg);
old_trans_split =
gnc_split_register_get_current_trans_split (reg, &old_trans_split_loc);
if (auto s{gnc_split_register_get_current_trans_split (reg, &old_trans_split_loc)};
is_trading_split(s))
old_trans_split = s;

old_class = gnc_split_register_get_current_cursor_class (reg);

exact_traversal = info->exact_traversal;
Expand Down

4 comments on commit 5c5b627

@christopherlam
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This breaks the register. Try "Show Splits" and tab/select the constituent splits.

@jralls
Copy link
Member Author

@jralls jralls commented on 5c5b627 Oct 1, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. Missed a ! on line 412. Fixed now.

@christopherlam
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There should also be a null check in the predicate, when moving from the blank split?

modified   gnucash/register/ledger-core/split-register-control.cpp
@@ -374,7 +374,8 @@ gnc_split_register_check_account (SplitRegister *reg,
 static inline bool
 is_trading_split (Split* split)
 {
-    return xaccAccountGetType (xaccSplitGetAccount (split)) == ACCT_TYPE_TRADING;
+    auto acct{xaccSplitGetAccount (split)};
+    return GNC_IS_ACCOUNT (acct) && xaccAccountGetType (acct) == ACCT_TYPE_TRADING;
 }
 
 static void

@jralls
Copy link
Member Author

@jralls jralls commented on 5c5b627 Oct 1, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea, stops it from spewing errors.

Please sign in to comment.