From f3fd025a35b7328f66e21db1eccfe774cae4cf70 Mon Sep 17 00:00:00 2001 From: Kunht Kun Date: Thu, 17 Mar 2022 01:09:33 -0400 Subject: Avoid making references to null In `compare_items` if `sort_order` is a top level expression, it has no context and `get_context()` method returns the null pointer. To see this, run $ ledger -f test/input/demo.ledger --sort display_amount reg --debug scope.symbols and there will be many lines like [DEBUG] Binding scope 0 with ... In such case making a reference to the context is an undefined behavior (honestly the dereferencing itself feels quite problematic, but many compilers just run without any complaints) and could potentially cause segfaults. Therefore, we change to use only the grandchild scope (`left` or `right`) for `find_sort_values` here. Note that it may seem to be more appropriate to use `report` here for the parent scope. However, in `find_sort_values` which is called right after, the `report` scope is always bound to this scope. So we only use grandchild scope to avoid unnecessary operations. Fixes #2069. --- src/compare.cc | 32 ++++++++++++++++++++++++-------- 1 file changed, 24 insertions(+), 8 deletions(-) (limited to 'src') diff --git a/src/compare.cc b/src/compare.cc index 63b85154..121f3014 100644 --- a/src/compare.cc +++ b/src/compare.cc @@ -88,15 +88,23 @@ bool compare_items::operator()(post_t * left, post_t * right) post_t::xdata_t& lxdata(left->xdata()); if (! lxdata.has_flags(POST_EXT_SORT_CALC)) { - bind_scope_t bound_scope(*sort_order.get_context(), *left); - find_sort_values(lxdata.sort_values, bound_scope); + if (sort_order.get_context()) { + bind_scope_t bound_scope(*sort_order.get_context(), *left); + find_sort_values(lxdata.sort_values, bound_scope); + } else { + find_sort_values(lxdata.sort_values, *left); + } lxdata.add_flags(POST_EXT_SORT_CALC); } post_t::xdata_t& rxdata(right->xdata()); if (! rxdata.has_flags(POST_EXT_SORT_CALC)) { - bind_scope_t bound_scope(*sort_order.get_context(), *right); - find_sort_values(rxdata.sort_values, bound_scope); + if (sort_order.get_context()) { + bind_scope_t bound_scope(*sort_order.get_context(), *right); + find_sort_values(rxdata.sort_values, bound_scope); + } else { + find_sort_values(rxdata.sort_values, *right); + } rxdata.add_flags(POST_EXT_SORT_CALC); } @@ -111,15 +119,23 @@ bool compare_items::operator()(account_t * left, account_t * right) account_t::xdata_t& lxdata(left->xdata()); if (! lxdata.has_flags(ACCOUNT_EXT_SORT_CALC)) { - bind_scope_t bound_scope(*sort_order.get_context(), *left); - find_sort_values(lxdata.sort_values, bound_scope); + if (sort_order.get_context()) { + bind_scope_t bound_scope(*sort_order.get_context(), *left); + find_sort_values(lxdata.sort_values, bound_scope); + } else { + find_sort_values(lxdata.sort_values, *left); + } lxdata.add_flags(ACCOUNT_EXT_SORT_CALC); } account_t::xdata_t& rxdata(right->xdata()); if (! rxdata.has_flags(ACCOUNT_EXT_SORT_CALC)) { - bind_scope_t bound_scope(*sort_order.get_context(), *right); - find_sort_values(rxdata.sort_values, bound_scope); + if (sort_order.get_context()) { + bind_scope_t bound_scope(*sort_order.get_context(), *right); + find_sort_values(rxdata.sort_values, bound_scope); + } else { + find_sort_values(rxdata.sort_values, *right); + } rxdata.add_flags(ACCOUNT_EXT_SORT_CALC); } -- cgit v1.2.3