1

I'm trying to write a method that changes font-size in a html string. When I click "minus" button, I multiply the current size with 0.8, when I click "plus" button, I multiply the current size with 1.25! The problem is at the htmlStyle.replace(pos, length, newString); line, at the 'newString' parameter. I figured out, that if I pass a string which contains a number in string format which was multiplied with for example 1.2, that's ok. But if I pass a number in string format which was multiplied with 1.22 or 1.34 or any floating point number with two decimal places, it causes an infinite loop and the app crashes. I really don't get what's the problem here, since these are strings, and problem occurs after the 'newString' was calculated successfully. Any ideas? I tried to figure this for 2 days, but I'm still clueless...

Here's the full method:

QString RichTextSize::setSizes(QString htmlStyle, float multiplier)
{
    QRegExp rx("(\\d+)pt");
    int pos = 0, length = 0;
    QString newString;

    while ((pos = rx.indexIn(htmlStyle, pos)) != -1) {
        length = rx.cap(1).length();
        newString.setNum(rx.cap(1).toInt()*multiplier);
        htmlStyle.replace(pos, length, newString);
        pos += rx.matchedLength();
    }
    return htmlStyle;
}

EDIT: I got a notification about a possible duplicate question. Well, my problem is not floating point math, but it's realted to float somehow. Floating point operations work without problems. When the trouble happens, that's with a float number converted to string. It converts successfully, but in the 'replace()' method it causes infinite loop. It's realted to floating point decimal places somehow, but since that operation works and problem is with a string converted from that number, I don't understand what happens here.

Alex
  • 39
  • 1
  • 8
  • probably floating point inaccuracy. e.g. your '1.2' would be "1.1912352978234892734" (or whatever) internally, and with multiple string/int conversions in there, could be getting truncated/rounded someplace you don't want it to go. – Marc B Jul 18 '16 at 19:47
  • 1
    Congratulations. You have just discovered that a) floating point values are not exact. b) floating point math has rounding errors. Go read this (for a start): https://docs.oracle.com/cd/E19957-01/806-3568/ncg_goldberg.html – Jesper Juhl Jul 18 '16 at 19:48
  • 1
    Possible duplicate of [Is floating point math broken?](http://stackoverflow.com/questions/588004/is-floating-point-math-broken) – Richard Critten Jul 18 '16 at 19:52

1 Answers1

2

You're using pos as an iterator on the string, yet by modifying the string you're invalidating the iterator. The loop is not guaranteed to terminate anymore. And, as you've found out, it doesn't.

The invalid assumption you're making, can be made explicit:

while ((pos = rx.indexIn(htmlStyle, pos)) != -1) {
    length = rx.cap(1).length();
    newString.setNum(rx.cap(1).toInt()*multiplier);
    htmlStyle.replace(pos, length, newString);
    Q_ASSERT(rx.matchedLength() == newString.size());
    pos += rx.matchedLength();
}

As soon as that assertion doesn't hold, the loop might not terminate anymore, or you might be accessing the string past its length, etc.

You should ideally iterate a constant string, so that the iterator stays valid, and build a new output string. This will be faster, too.

Another alternative is to fix the iterator update so that it doesn't get invalidated. Hint: pos += rx.matchedLength() is based on an invalidated part of the string: it was true right before the htmlStyle.replace, but isn't anymore. Perhaps you meant pos += newString.size().

Kuba hasn't forgotten Monica
  • 95,931
  • 16
  • 151
  • 313
  • Thank you very much! This solved my problem and works well. Looks like I was blind to see the obvious... But it still not answers why my code was working without problems if I used float values with only one decimal places. It was producing the same length of new data, so it shouldn't be working. Am I right? As I said, this problem occured only when float values with two decimal places were used. What's the connection? – Alex Jul 18 '16 at 23:32
  • 1
    @Allex The "connection" is called "coincidence". You were lucky: when you used one decimal place, the replacement strings in your particular circumstances happened to have same length and didn't invalidate your iterator. There's nothing special much about one decimal place, it was just a coincidence. The takeaway here is to mind your iterators and not invalidate them. The question is only tangentially related to floating point. You don't need floating point to demonstrate the same problem. You could simply generate a random-length `newString` full of spaces - no need for float-anything! – Kuba hasn't forgotten Monica Jul 19 '16 at 14:56
  • Thanks for the comment! But it must be something with float. I was modifying font-size in a html string. From "font-size:11pt" I took "11" multiplied it and replaced it with the result. When I was multiplying with 1.2, the result was 13.2 and that's longer than 11, but there wasn't any problem. In the meantime I was experimenting, and replaced "11" with a contant string "HAHAHAHAA". That's also longer than "11", but it was working like charm... The problem only occured when I was multiplying with a float with two decimal places. Am I really this stupid or is it really a mistery? – Alex Jul 19 '16 at 17:23
  • @Alex Why are you citing cases where it worked where obviously you know of the case(s) where it doesn't work? The float is immaterial, the strings *are*! If you don't believe me, dump out the state of the loop `qDebug() << pos << rx.cap(1) << newString;` and see for yourself exactly where it fails. You can use any strings you want as replacements, as long as they have the length that makes it fail, fail it will. I can reproduce your issue on an ARMv6 board with disabled floating point hardware and no emulation. Really. – Kuba hasn't forgotten Monica Jul 19 '16 at 18:10
  • Thanks for the help! I put some more things into the debug: `qDebug() << pos << rx.cap(1) << rx.cap(1).size() << newString << newString.size() << rx.matchedLength();` and this gave me the perfect answer now. I'm still studying, I'm learning programming only for a couple of years and I never faced a problem like this until now, so I needed a little education. – Alex Jul 19 '16 at 19:52
  • 1
    @Alex Remember though that your problem is from a broad class one might call *iterator invalidation fallout*. Whenever you write a loop that iterates some container, always mind whether your iterator remains valid in face of any modification to the container. An iterator is an abstract concept, it doesn't have to be any particular iterator type. It can be an enum, an integer, a pointer, ... C++ standard containers are decently documented as to guarantee when their iterators remain valid. When you use an integer index, you will have to understand what you're doing and when it might get invalid. – Kuba hasn't forgotten Monica Jul 19 '16 at 21:47