| Summary: | KMyMoney instantly crashes upon assigning "Input Column" numbers during CSV file import | ||
|---|---|---|---|
| Product: | [Applications] kmymoney | Reporter: | at |
| Component: | general | Assignee: | KMyMoney Devel Mailing List <kmymoney-devel> |
| Status: | RESOLVED FIXED | ||
| Severity: | crash | CC: | agander93, ostroffjh |
| Priority: | NOR | ||
| Version: | 4.6.2 | ||
| Target Milestone: | --- | ||
| Platform: | Fedora RPMs | ||
| OS: | Linux | ||
| Latest Commit: | http://commits.kde.org/kmymoney/a83c75715d3e5ba2037cd2aa1b38a97f043b9a40 | Version Fixed In: | |
| Sentry Crash Report: | |||
| Attachments: | A sample CSV file, including the header and a single line of data. | ||
|
Description
at
2012-08-22 04:06:41 UTC
(In reply to comment #0) > Application: kmymoney (4.6.2) > KDE Platform Version: 4.8.5 (4.8.5) > Qt Version: 4.8.2 > Operating System: Linux 3.5.1-1.fc17.x86_64 x86_64 > Distribution (Platform): Fedora RPMs > > -- Information about the crash: > - What I was doing when the application crashed: > > 1) Choose to 'Import CSV' 2) Open target CSV file 3) Assign ANY "Input > Column" number, by merely clicking on a number in the drop down menu = > instant crash. > > The crash can be reproduced every time. > > -- Backtrace: > Application: KMyMoney (kmymoney), signal: Segmentation fault > Using host libthread_db library "/lib64/libthread_db.so.1". > [Current thread is 1 (Thread 0x7f52f1ad5880 (LWP 17516))] > > Thread 1 (Thread 0x7f52f1ad5880 (LWP 17516)): > [KCrash Handler] > #6 0x0000003d1f0c0466 in QString::operator== (this=0x46d5f80, other=...) at > tools/qstring.cpp:2192 > #7 0x00007f52e226702d in CsvImporterDlg::validateColumn > (this=this@entry=0x46d5b50, col=@0x7fff56189f6c: 9, type=...) at > /usr/src/debug/kmymoney-4.6.2/kmymoney/plugins/csvimport/csvimporterdlg.cpp: > 259 > #8 0x00007f52e2267220 in CsvImporterDlg::numberColumnSelected > (this=0x46d5b50, col=9) at > /usr/src/debug/kmymoney-4.6.2/kmymoney/plugins/csvimport/csvimporterdlg.cpp: > 452 Obviously, this does not happen normally, so it may be that there is something in your file format that is not being handled correctly. You do not need to send the whole file if it is large, as long as the line on which it chokes is included, if you you can determine this. Might one of the lines have fewer cells than the other lines? Is it possible to provide a copy of your file with any private data disguised, as long as the format is not changed. Are you using the stock Fedora package, or have you compiled from source? Created attachment 73403 [details]
A sample CSV file, including the header and a single line of data.
The data I am trying to import is a typical PayPal history download (in CSV format). The file I have attached is an example of their CSV format. I have edited it to have only one row of data and, plus the header row. This file also causes an instant crash upon trying to assign and "Input Column". I am using stock Fedora 17 x86_64. Although, I have also reproduced the problem on stock Fedora 15 x86_64, as well. (In reply to comment #3) > The data I am trying to import is a typical PayPal history download (in CSV > format). The file I have attached is an example of their CSV format. I have > edited it to have only one row of data and, plus the header row. This file > also causes an instant crash upon trying to assign and "Input Column". > > I am using stock Fedora 17 x86_64. Although, I have also reproduced the > problem on stock Fedora 15 x86_64, as well. Confirmed! That sets a world record for the greatest number of columns, 45. I'd only allowed for 25, which itself is greater than I've seen. An array is going out-of-bounds. Need to be cleverer. It may take a while for the distros to pick up the fix, I'm afraid, unless you are able to compile from source? Temporarily, I've changed the default limit to 50 columns, and your file now imports. How long before a 51-column file appears? I'll need to look more closely. Even inf you can't make the number of columns completely dynamic, can you at least check the number of columns and issue an error, rather than crashing? (In reply to comment #6) > Even inf you can't make the number of columns completely dynamic, can you at > least check the number of columns and issue an error, rather than crashing? I'd sooner not have to do anything major, but I'm aiming for dynamic. An error message doesn't do much for the user's import, but....we'll see. Please try to use any of the Qt container classes. it should help you with the dynamics a lot. QVector sounds like a good candidate. See http://qt-project.org/doc/qt-4.8/containers.html for an overview. (In reply to comment #8) > Please try to use any of the Qt container classes. it should help you with > the dynamics a lot. QVector sounds like a good candidate. See > http://qt-project.org/doc/qt-4.8/containers.html for an overview. I think the reference to 'dynamic' was perhaps a bit misleading. What was being referred to was to replace the original fixed array size with one depending on the file size. The actual array itself requires little manipulation, no insertions or deletions, apart from when a ne file is loaded. I looked at QVector and didn't see anything there that was not provided by replacing the QString array with a QStringList. The major problem was in the relocation of the 'setting sizes' code away from the ctor to the readFile routine. This in turn lead to a number of connect problems. All now seems to be well, both for banking and investments. The changes are not insignificant - kmymoney/plugins/csvimport/csvdialog.cpp | 308 ++++++++++++----------- kmymoney/plugins/csvimport/csvdialog.h | 5 +- kmymoney/plugins/csvimport/investmentdlg.cpp | 32 --- kmymoney/plugins/csvimport/investmentdlg.h | 7 - kmymoney/plugins/csvimport/investprocessing.cpp | 231 +++++++++++------- kmymoney/plugins/csvimport/investprocessing.h | 5 +- 6 files changed, 316 insertions(+), 272 deletions(-) So, do I need to go through reviewboard? At the moment, the fixes apply to the stable release only. The reason is that I am some way through a biggish tidy-up of the development version, and am intending to include the bug fix in with them. PS Bearing in mind what Cristian has said about not making reviewboard code too large, it might not be such a good idea to merge these changes in with another biggish set. So, should I instead apply them to the HEAD? This might also give them a bit more visibility too. Then. backport to stable. Please try to seperate the two actions into two different git change sets (commits) so that we can backport it using cherry-picking. (In reply to comment #11) > Please try to seperate the two actions into two different git change sets > (commits) so that we can backport it using cherry-picking. I've now applied the changes locally to HEAD, and I'll do the commit later. Git commit a83c75715d3e5ba2037cd2aa1b38a97f043b9a40 by Allan Anderson. Committed on 28/08/2012 at 12:27. Pushed by allananderson into branch 'master'. Fix handling of multiple columns M +160 -130 kmymoney/plugins/csvimport/csvdialog.cpp M +2 -3 kmymoney/plugins/csvimport/csvdialog.h M +0 -32 kmymoney/plugins/csvimport/investmentdlg.cpp M +0 -9 kmymoney/plugins/csvimport/investmentdlg.h M +145 -87 kmymoney/plugins/csvimport/investprocessing.cpp M +2 -4 kmymoney/plugins/csvimport/investprocessing.h M +0 -1 kmymoney/plugins/csvimport/redefinedlg.cpp M +0 -2 kmymoney/plugins/csvimport/redefinedlg.h http://commits.kde.org/kmymoney/a83c75715d3e5ba2037cd2aa1b38a97f043b9a40 |