iamantony / qtcsv

Library for reading and writing csv-files in Qt.
MIT License
266 stars 144 forks source link

Bug in Qt 5.12 which appears when we use QtCsv #55

Closed SzykCech4 closed 5 years ago

SzykCech4 commented 5 years ago

Hi! Please check attached project. You have set Qt5.12 as your chosen Qt version and run app in debug mode (F5). Then program will hang in main() function on the first line (of the two). I report you this problem, because I am unable to determine reasons of this hang. This is not happen in Qt5.9.7. I guess this bug is related to they "great speed improvement" in 5.12 release (as you can see there are some SSE code around). Please take a look in debuger output and give me a hint how describe this bug to the Qt (or better: you can report it by yourself). QtCsvAndQt512Bug.zip

iamantony commented 5 years ago

Hello @SzykCech4 ! Thanks for your interest in qtcsv. Unfortunately I don't have much free time now to help you with this issue. So any help from community will be highly welcomed!

SzykCech4 commented 5 years ago

Ok. I investigated code for a while and can some thing to tell: It is QtCsv bug and Qt bug (two bugs). QtCsv bug is in ReaderPrivate::removeExtraSymbols() function in reader.cpp: This code hangs:

            QStringRef strStart(&elements.at(i), startPos, textDelimiter.size());
            if ( strStart == textDelimiter)
            {
                startPos += textDelimiter.size();
            }

            // Skip text delimiter symbol if element ends with it
            QStringRef strEnd(&elements.at(i), endPos - textDelimiter.size() + 1,
                              textDelimiter.size());
            if (strEnd == textDelimiter)
            {
                endPos -= textDelimiter.size();
            }

Problem is that you assume every cells have some data, but this is not true. When you have empty cell then strStart will point to the \0 character at the end of the string (strEnd is even worst because it will point to the -1 character). Please be so kindly and fix this. Mod and tests should give you about an hour... Of the Qt site bug was reported here: Qt bug report

thiagomacieira commented 5 years ago

I've closed the Qt issue. Analysis determines QtCsv is violating QStringRef invariants, by shrinking the QString that it points to.

I highly recommend using QStringView in new code.

iamantony commented 5 years ago

Thank you @SzykCech4 and @thiagomacieira ! I'll try to fix it asap.

iamantony commented 5 years ago

@SzykCech4 finally I was able to check what was the problem here. In your example project you use qtcsv 1.5.0. This version does not contain check for an empty string in function ReaderPrivate::removeExtraSymbols(), that was added about a year ago in master branch.

So solution here:

@SzykCech4 can I use your csv file in my tests?

SzykCech4 commented 5 years ago

Thank you for your work.

@SzykCech4 can I use your csv file in my tests?

Yes you can.

iamantony commented 5 years ago

Thanks @SzykCech4 ! Closing the bug.