fixed two memory corruption bugs

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<commodities_map::iterator, bool> 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<std::string>::const_iterator begin,
@@ -140,10 +141,8 @@ regexps_to_predicate(std::list<std::string>::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_t> 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<entry_t> 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 <typename T>
 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;
   }
 };