Summary: | Floating point crash importing gnucash file | ||
---|---|---|---|
Product: | [Applications] kmymoney | Reporter: | Bradley Baetz <bbaetz> |
Component: | general | Assignee: | KMyMoney Devel Mailing List <kmymoney-devel> |
Status: | RESOLVED FIXED | ||
Severity: | crash | CC: | mytorciar, wayne |
Priority: | NOR | ||
Version: | unspecified | ||
Target Milestone: | --- | ||
Platform: | Fedora RPMs | ||
OS: | Linux | ||
Latest Commit: | Version Fixed In: | ||
Attachments: | Minimal test case |
Description
Bradley Baetz
2010-11-07 09:09:29 UTC
Could you attach a GNUCash test file to reproduce this. If it happens every time it should be easy to reproduce it with a test file. Created attachment 53433 [details]
Minimal test case
Minimal file attached - taken by cutting down my actual file as much as possible.
Removing the 'price' that has a value of zero fixes this, both on the test case and my original file
The bug is caused by the price having the value 0. The price is used later on in a conversion to the base currency which causes the crash. There are two ways to fix this: 1) Don't allow the import of a price that has the value 0 because it doesn't really make sense to have a conversion rate of 0 (KMyMoney's UI does not allow entering a price with the value 0 - I wonder about other price sources though such as online quotes). 2) Skip the conversion if the rate is 0; But with what kind of result? If let's say 1 A = 0 B then how many A's would 1 B value? Personally I prefer solution 1. I posted this here so the others can have their say on how to fix this. Thanks for the file. FWIW, this was an auto-downloaded online quote (stock had changed its code and I hadn't updated gnucash) OTOH, 0 is 'valid' as a price; companies do go under. Thomas, Alvaro, Tony any thoughts on how should we handle this? Seems that price equals 0 is a valid condition. I propose the following patch: Index: accountsmodel.cpp =================================================================== --- accountsmodel.cpp (revision 1204831) +++ accountsmodel.cpp (working copy) @@ -241,12 +241,16 @@ { QList<MyMoneyPrice>::const_iterator it_p; QString security = account.currencyId(); - for (it_p = prices.constBegin(); it_p != prices.constEnd(); ++it_p) { - value = (value * (MyMoneyMoney(1, 1) / (*it_p).rate(security))).convert(MyMoneyMoney::precToDenom(KMyMoneyGlobalSettings::pricePrecision())); - if ((*it_p).from() == security) - security = (*it_p).to(); - else - security = (*it_p).from(); + for (it_p = prices.constBegin(); !value.isZero() && it_p != prices.constEnd(); ++it_p) { + if((*it_p).rate(security).isZero()) { + value = 0; + } else { + value = (value * (MyMoneyMoney(1, 1) / (*it_p).rate(security))).convert(MyMoneyMoney::precToDenom(KMyMoneyGlobalSettings::pricePrecision())); + if ((*it_p).from() == security) + security = (*it_p).to(); + else + security = (*it_p).from(); + } } value = value.convert(m_file->baseCurrency().smallestAccountFraction()); } If price equals 0, reports will crash. We've been through that before. So, if we decide to allow prices in 0 to go in, there's a lot more to change. I was about to suggest to look for other places in the code where such conversion is performed but Alvaro said it all. If the UI dos not allow entering a conversion rate of 0 I would suggest forbidding it during import also. The other solution would be to check everywhere a conversion rate is used and guard it against a zero value but then we have to implement the ??? value also (shown in the UI for the inverse conversion when you enter 0 as a price for direct conversion). SVN commit 1205765 by tbaumgart: Don't import zero prices from GnuCash Print more details about the price to the console BUG: 256282 M +2 -0 converter/mymoneygncreader.cpp M +2 -1 mymoney/mymoneyprice.cpp WebSVN link: http://websvn.kde.org/?view=rev&revision=1205765 SVN commit 1205768 by tbaumgart: Don't import zero prices from GnuCash Print more details about the price to the console Backported to stable branch BUG: 256282 M +2 -0 converter/mymoneygncreader.cpp M +2 -1 mymoney/mymoneyprice.cpp WebSVN link: http://websvn.kde.org/?view=rev&revision=1205768 *** Bug 264312 has been marked as a duplicate of this bug. *** *** Bug 264313 has been marked as a duplicate of this bug. *** *** Bug 274656 has been marked as a duplicate of this bug. *** |