[Libreoffice] [REVIEW] fix for a chart crasher
hunteke at earlham.edu
Thu Mar 3 06:42:38 PST 2011
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 /
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 *
If it's 495, I don't know how it's getting past the if above.
More information about the LibreOffice