Date: prev next · Thread: first prev next last
2011 Archives by date, by thread · List index


At 4:22am -0500 Thu, 03 Mar 2011, Tor Lillqvist wrote:
for a cherry-pick candidate to libreoffice-3-3. It's a very simple&
safe fix, and fixes a crasher as reported in

But do you know how those values can end up being negative in the
first place, is that a symptom of something being horribly wrong
otherwise, and this patch now then just plasters over that and
instead of crashing it might permit silent data corruption elsewhere
then instead?

Alas, in my limited 20 minutes last night I was not able to get the full scope in my mind, so I pose it as a question here instead: I'm a little concerned with /why/ either of those two functions are returning a negative number at all. (I certainly didn't give LO one in the input!) Is there ever a time that either of those should be negative? The rest of the code in getAllTicks() assumes that they're always >= 0.

At first glance, getTickDepth() and getMaxTickCount() read to me as requests for non-negative values (from an English/math sense). However, I note that the return values are sal_Int32 instead of sal_uInt32.

Going one call down in the stack trace:

sal_Int32 TickmarkHelper::getTickDepth() const
{
    return m_rIncrement.SubIncrements.getLength() + 1;
}

If that's truly a length (where are the frickin' data definitions?!?!), then, sans a wraparound, that certainly shouldn't be negative. Surely we aren't at MAX_INT?

sal_Int32 TickmarkHelper::getMaxTickCount( sal_Int32 nDepth ) const

Again, I've run out of time, but my spidey sense is pointing to a combination of "too small a range" = close enough to zero for the computer to interpret as 0 or returning a negative "almost 0", and then line 484 or 495:

484: sal_Int32 nIntervalCount = static_cast<sal_Int32>( fSub / m_rIncrement.Distance );

I have not yet sussed out what approxSub() or isFinite() does to fSub. Are they making nIntervalCount negative, through the fSub proxy?

495: nTickCount = nIntervalCount * (m_rIncrement.SubIncrements[nDepth-1].IntervalCount-1);

If it's 495, I don't know how it's getting past the if above.

Thoughts?

Kevin

Context


Privacy Policy | Impressum (Legal Info) | Copyright information: Unless otherwise specified, all text and images on this website are licensed under the Creative Commons Attribution-Share Alike 3.0 License. This does not include the source code of LibreOffice, which is licensed under the Mozilla Public License (MPLv2). "LibreOffice" and "The Document Foundation" are registered trademarks of their corresponding registered owners or are in actual use as trademarks in one or more countries. Their respective logos and icons are also subject to international copyright laws. Use thereof is explained in our trademark policy.