From ac000a67c4e9c75e11e43018e5f16242b4d99226 Mon Sep 17 00:00:00 2001 From: John Wiegley Date: Wed, 25 Aug 2004 21:02:07 -0400 Subject: fixed two memory corruption bugs --- Makefile.am | 2 +- amount.cc | 88 +++++++++++++++++++++++++++++++++++++++---------------------- balance.h | 15 ++++++++--- binary.cc | 7 +++-- binary.h | 1 + ledger.cc | 5 ++++ ledger.el | 11 +++++--- main.cc | 28 +++++++++++++------- walk.h | 7 ++--- 9 files changed, 110 insertions(+), 54 deletions(-) diff --git a/Makefile.am b/Makefile.am index c6f7fd33..2b8c6c1f 100644 --- a/Makefile.am +++ b/Makefile.am @@ -12,7 +12,7 @@ endif bin_PROGRAMS = ledger if DEBUG -ledger_CXXFLAGS = -DDEBUG_LEVEL=4 -DDO_CLEANUP +ledger_CXXFLAGS = -DDEBUG_LEVEL=4 endif ledger_SOURCES = main.cc ledger_LDADD = $(LIBOBJS) libledger.a diff --git a/amount.cc b/amount.cc index dd410f50..b72f9852 100644 --- a/amount.cc +++ b/amount.cc @@ -66,13 +66,38 @@ void initialize_amounts() commodity_t::null_commodity = commodity_t::find_commodity("", true); } +void clean_commodity_history(char * item_pool, char * item_pool_end) +{ + for (commodities_map::iterator i = commodity_t::commodities.begin(); + i != commodity_t::commodities.end(); + i++) + for (history_map::iterator j = (*i).second->history.begin(); + j != (*i).second->history.end(); + j++) { + amount_t::bigint_t * quantity = (*j).second.quantity; + if (quantity && + (char *)quantity >= item_pool && + (char *)quantity < item_pool_end) { + assert(quantity->flags & BIGINT_BULK_ALLOC); + + // Since the journal in which this price was bulk alloc'd (on + // reading from a binary file) is going away, we must make a + // new copy of the value, which other journals might still be + // using. + + amount_t::bigint_t * q = new amount_t::bigint_t(*quantity); + if (--quantity->ref == 0) + quantity->~bigint_t(); + (*j).second.quantity = q; + } + } +} + void shutdown_amounts() { mpz_clear(divisor); mpz_clear(temp); - true_value.ref--; - if (commodity_t::updater) { delete commodity_t::updater; commodity_t::updater = NULL; @@ -84,6 +109,8 @@ void shutdown_amounts() delete (*i).second; commodity_t::commodities.clear(); + + true_value.ref--; } static void mpz_round(mpz_t out, mpz_t value, int value_prec, int round_prec) @@ -499,30 +526,32 @@ AMOUNT_CMP_UINT(>=) AMOUNT_CMP_UINT(==) // comparisons between amounts -#define AMOUNT_CMP_AMOUNT(OP) \ -bool amount_t::operator OP(const amount_t& amt) const \ -{ \ - if (! quantity) \ - return amt > 0; \ - if (! amt.quantity) \ - return *this < 0; \ - \ - if (commodity != amt.commodity) \ - throw amount_error("Comparing amounts with different commodities"); \ - \ - if (quantity->prec == amt.quantity->prec) { \ - return mpz_cmp(MPZ(quantity), MPZ(amt.quantity)) OP 0; \ - } \ - else if (quantity->prec < amt.quantity->prec) { \ - amount_t temp = *this; \ - temp._resize(amt.quantity->prec); \ - return mpz_cmp(MPZ(temp.quantity), MPZ(amt.quantity)) OP 0; \ - } \ - else { \ - amount_t temp = amt; \ - temp._resize(quantity->prec); \ - return mpz_cmp(MPZ(quantity), MPZ(temp.quantity)) OP 0; \ - } \ +#define AMOUNT_CMP_AMOUNT(OP) \ +bool amount_t::operator OP(const amount_t& amt) const \ +{ \ + if (! quantity) \ + return amt > 0; \ + if (! amt.quantity) \ + return *this < 0; \ + \ + if (commodity != amt.commodity && \ + commodity != commodity_t::null_commodity && \ + amt.commodity != commodity_t::null_commodity) \ + return false; \ + \ + if (quantity->prec == amt.quantity->prec) { \ + return mpz_cmp(MPZ(quantity), MPZ(amt.quantity)) OP 0; \ + } \ + else if (quantity->prec < amt.quantity->prec) { \ + amount_t temp = *this; \ + temp._resize(amt.quantity->prec); \ + return mpz_cmp(MPZ(temp.quantity), MPZ(amt.quantity)) OP 0; \ + } \ + else { \ + amount_t temp = amt; \ + temp._resize(quantity->prec); \ + return mpz_cmp(MPZ(quantity), MPZ(temp.quantity)) OP 0; \ + } \ } AMOUNT_CMP_AMOUNT(<) @@ -832,7 +861,6 @@ void amount_t::parse(const std::string& str) static char buf[4096]; -static int index = 0; void amount_t::write_quantity(std::ostream& out) const { @@ -845,7 +873,7 @@ void amount_t::write_quantity(std::ostream& out) const } if (quantity->index == 0) { - quantity->index = ++index; + quantity->index = ++bigints_index; bigints_count++; byte = 1; @@ -906,8 +934,6 @@ void amount_t::read_quantity(std::istream& in) in.read((char *)&index, sizeof(index)); quantity = bigints + (index - 1); quantity->ref++; - DEBUG_PRINT("ledger.amount.bigint-show", - "bigint " << quantity << " ++ref " << quantity->ref); } } @@ -930,7 +956,7 @@ bool amount_t::valid() const void commodity_t::add_price(const std::time_t date, const amount_t& price) { - history_map::const_iterator i = history.find(date); + history_map::iterator i = history.find(date); if (i != history.end()) { (*i).second = price; } else { diff --git a/balance.h b/balance.h index 550ecb28..7f547e8d 100644 --- a/balance.h +++ b/balance.h @@ -39,19 +39,26 @@ class balance_t } balance_t(const amount_t& amt) { DEBUG_PRINT("ledger.memory.ctors", "ctor balance_t"); - *this += amt; + if (amt) + amounts.insert(amounts_pair(amt.commodity, amt)); } balance_t(const int value) { DEBUG_PRINT("ledger.memory.ctors", "ctor balance_t"); - *this += amount_t(value); + amount_t amt(value); + if (amt) + amounts.insert(amounts_pair(amt.commodity, amt)); } balance_t(const unsigned int value) { DEBUG_PRINT("ledger.memory.ctors", "ctor balance_t"); - *this += amount_t(value); + amount_t amt(value); + if (amt) + amounts.insert(amounts_pair(amt.commodity, amt)); } balance_t(const double value) { DEBUG_PRINT("ledger.memory.ctors", "ctor balance_t"); - *this += amount_t(value); + amount_t amt(value); + if (amt) + amounts.insert(amounts_pair(amt.commodity, amt)); } // destructor diff --git a/binary.cc b/binary.cc index 5f5bc1aa..0d71a11b 100644 --- a/binary.cc +++ b/binary.cc @@ -9,7 +9,7 @@ namespace ledger { const unsigned long binary_magic_number = 0xFFEED765; -static const unsigned long format_version = 0x00020018; +static const unsigned long format_version = 0x00020019; static account_t ** accounts; static account_t ** accounts_next; @@ -21,6 +21,7 @@ static unsigned int commodity_index; amount_t::bigint_t * bigints; amount_t::bigint_t * bigints_next; +unsigned int bigints_index; unsigned int bigints_count; #if DEBUG_LEVEL >= ALPHA @@ -287,11 +288,13 @@ unsigned int read_binary_journal(std::istream& in, std::size_t pool_size = (sizeof(entry_t) * count + sizeof(transaction_t) * xact_count + sizeof_bigint_t() * bigint_count); + char * item_pool = new char[pool_size]; entry_t * entry_pool = (entry_t *) item_pool; transaction_t * xact_pool = (transaction_t *) (item_pool + sizeof(entry_t) * count); + bigints_index = 0; bigints = bigints_next = (amount_t::bigint_t *) (item_pool + sizeof(entry_t) * count + sizeof(transaction_t) * xact_count); @@ -305,7 +308,7 @@ unsigned int read_binary_journal(std::istream& in, std::pair result = commodity_t::commodities.insert(commodities_pair(commodity->symbol, commodity)); - assert(result.second || master); + assert(result.second); } // Read in the entries and transactions diff --git a/binary.h b/binary.h index 187cdcb8..2cc4b5b4 100644 --- a/binary.h +++ b/binary.h @@ -20,6 +20,7 @@ class binary_parser_t : public parser_t extern amount_t::bigint_t * bigints; extern amount_t::bigint_t * bigints_next; +extern unsigned int bigints_index; extern unsigned int bigints_count; void write_binary_journal(std::ostream& out, diff --git a/ledger.cc b/ledger.cc index d2a3bf15..3a6f1016 100644 --- a/ledger.cc +++ b/ledger.cc @@ -68,6 +68,8 @@ bool entry_t::valid() const return true; } +void clean_commodity_history(char * item_pool, char * item_pool_end); + journal_t::~journal_t() { DEBUG_PRINT("ledger.memory.dtors", "dtor journal_t"); @@ -86,6 +88,9 @@ journal_t::~journal_t() else (*i)->~entry_t(); + // Remove historical prices which were allocated in the item_pool. + clean_commodity_history(item_pool, item_pool_end); + if (item_pool) delete[] item_pool; } diff --git a/ledger.el b/ledger.el index 9ae2637c..416026b0 100644 --- a/ledger.el +++ b/ledger.el @@ -119,11 +119,14 @@ (with-temp-buffer (setq exit-code (apply 'ledger-run-ledger "entry" args)) (if (= 0 exit-code) - (if insert-year - (buffer-string) - (buffer-substring 5 (point-max))) + (progn + (goto-char (point-min)) + (delete-char 1) + (if insert-year + (buffer-string) + (buffer-substring 5 (point-max)))) (concat (if insert-year entry - (substring entry 5)) "\n\n"))))))) + (substring entry 5)) "\n"))) "\n")))) (defun ledger-expand-entry () (interactive) diff --git a/main.cc b/main.cc index a9f30fd8..0aca49f0 100644 --- a/main.cc +++ b/main.cc @@ -13,6 +13,7 @@ #include "quotes.h" #include "option.h" #include "config.h" +#include "debug.h" #include "timing.h" #include "error.h" @@ -42,7 +43,7 @@ namespace { TIMER_DEF(read_cache, "reading cache file"); } -#ifndef DO_CLEANUP +#if !defined(DEBUG_LEVEL) || DEBUG_LEVEL <= RELEASE #define auto_ptr bogus_auto_ptr @@ -74,7 +75,7 @@ namespace std { }; } -#endif // !DO_CLEANUP +#endif static void regexps_to_predicate(std::list::const_iterator begin, @@ -140,10 +141,8 @@ regexps_to_predicate(std::list::const_iterator begin, } } -int main(int argc, char * argv[], char * envp[]) +int parse_and_report(int argc, char * argv[], char * envp[]) { - initialize(); - std::auto_ptr journal(new journal_t); // Initialize the global configuration object for this run @@ -271,6 +270,8 @@ int main(int argc, char * argv[], char * envp[]) return 1; } + VALIDATE(journal->valid()); + TIMER_STOP(parse_files); // Read the command word, and then check and simplify it @@ -320,6 +321,8 @@ int main(int argc, char * argv[], char * envp[]) std::auto_ptr new_entry; if (command == "e") { new_entry.reset(journal->derive_entry(arg, args.end())); + if (! new_entry.get()) + return 1; } else { // Treat the remaining command-line arguments as regular // expressions, used for refining report results. @@ -612,7 +615,7 @@ int main(int argc, char * argv[], char * envp[]) acct_formatter.flush(); } -#ifdef DO_CLEANUP +#if DEBUG_LEVEL >= BETA // Cleanup the data handlers that might be present on some objects. clear_transaction_data xact_cleanup; @@ -635,13 +638,20 @@ int main(int argc, char * argv[], char * envp[]) TIMER_STOP(write_cache); -#ifdef DO_CLEANUP - VALIDATE(journal->valid()); + return 0; +} + +int main(int argc, char * argv[], char * envp[]) +{ + initialize(); + int status = parse_and_report(argc, argv, envp); + +#if DEBUG_LEVEL >= BETA shutdown(); #endif - return 0; + return status; } // main.cc ends here. diff --git a/walk.h b/walk.h index 0e7d2e5e..238fdb78 100644 --- a/walk.h +++ b/walk.h @@ -40,9 +40,6 @@ struct item_handler { template class compare_items { - value_t left_result; - value_t right_result; - const value_expr_t * sort_order; public: @@ -54,8 +51,12 @@ class compare_items { bool operator()(const T * left, const T * right) { assert(left); assert(right); + + value_t left_result; sort_order->compute(left_result, details_t(left)); + value_t right_result; sort_order->compute(right_result, details_t(right)); + return left_result < right_result; } }; -- cgit v1.2.3