Bugs (?) in gnc-pricedb.c

[Mike Alexander]

--On February 19, 2006 2:01:01 PM -0800 Andrew Sackville-West 
<andrew at farwestbilliards.com> wrote:

> On Wed, 08 Feb 2006 02:50:47 -0500
> Mike Alexander <mta at umich.edu> wrote:
>
>> While looking for another problem I found what look like bugs in a
>> couple of functions [1] in gnc-pricedb.c.  It appears to me that the
>> loop termination test is backwards in a couple of places.  I've
>> attached a patch that fixes this (if it is indeed a problem).
>>
>> I added a number of patches to bug reports, but since I don't know
>> how  to cause this bug to occur I didn't create a bug report for it.
>>
>> [1] gnc_pricedb_convert_balance_latest_price and
>> gnc_pricedb_convert_balance_nearest_price.
>
> Hampton,
>
> I see other problems with these functions...
> for ex: if (!price-list) { balance = gnc_numeric_zero; return
> balance;};
>
> well, that just doesn'tdoens't make sense. just because there is no
> price-list we return a 0 for the balance? granted the function has
> pricedb in the name and so its reasonable to assume there is a
> pricelist, but how do you distinguish between a no price-list
> situation and a balance that is actually zero?

I found another problem in this code tonight.  The price lookup 
functions try to handle the case of converting currency1 into currency2 
if the pricedb contains an exchange rate for currency2->currency1 but 
not vice versa (by using the reciprocal).  It did it right in the hard 
case where you give it a commodity priced in currency1 and ask for the 
value in currency2, but not for the easier case where you give it a 
value in currency1 and ask for it in currency2.  The attached patch 
makes it handle that case too.  It also contains the previous loop 
termination patch, which I'm now quite sure is correct.  It does not do 
anything about the bug you mentioned above since I didn't see this 
message until after I was done with this.

-- 
Mike Alexander           mta at umich.edu
Ann Arbor, MI            PGP key ID: BEA343A6

-------------- next part --------------
Index: src/engine/gnc-pricedb.c
===================================================================
--- src/engine/gnc-pricedb.c	(revision 13293)
+++ src/engine/gnc-pricedb.c	(working copy)
@@ -1721,6 +1721,18 @@
     return balance;
   }
 
+  /* Look for a price of the new currency in the balance currency and use
+   * the reciprocal if we find it 
+   */
+  price = gnc_pricedb_lookup_latest (pdb, new_currency, balance_currency);
+  if (price) {
+    balance = gnc_numeric_div (balance, gnc_price_get_value (price),
+                               gnc_commodity_get_fraction (new_currency),
+                               GNC_HOW_RND_ROUND);
+    gnc_price_unref (price);
+    return balance;
+  }
+
   /*
    * no direct price found, try if we find a price in another currency
    * and convert in two stages
@@ -1758,7 +1770,7 @@
 
     list_helper = list_helper->next;
   } while((list_helper != NULL) &&
-          (!gnc_numeric_zero_p(currency_price_value)));
+          (gnc_numeric_zero_p(currency_price_value)));
 
   balance = gnc_numeric_mul (balance, currency_price_value,
                              gnc_commodity_get_fraction (new_currency),
@@ -1797,6 +1809,18 @@
     return balance;
   }
 
+  /* Look for a price of the new currency in the balance currency and use
+   * the reciprocal if we find it 
+   */
+  price = gnc_pricedb_lookup_nearest_in_time (pdb, new_currency, balance_currency, t);
+  if (price) {
+    balance = gnc_numeric_div (balance, gnc_price_get_value (price),
+                               gnc_commodity_get_fraction (new_currency),
+                               GNC_HOW_RND_ROUND);
+    gnc_price_unref (price);
+    return balance;
+  }
+
   /*
    * no direct price found, try if we find a price in another currency
    * and convert in two stages
@@ -1834,7 +1858,7 @@
 
     list_helper = list_helper->next;
   } while((list_helper != NULL) &&
-          (!gnc_numeric_zero_p(currency_price_value)));
+	  (gnc_numeric_zero_p(currency_price_value)));
 
   balance = gnc_numeric_mul (balance, currency_price_value,
                              gnc_commodity_get_fraction (new_currency),