[Libreoffice] [PATCH] ScGlobal::GetAutoFormat not always required to create fresh instance
Stephan Bergmann
sbergman at redhat.com
Fri Jan 6 10:03:05 PST 2012
The number one reason why "make check" fails for me is a SEGV in
soffice.bin during termination of JunitTest_sc_unoapi, namely at
> #0 0x00007f4906ecc0d2 in ScGlobal::GetRscString (nIndex=71) at sc/source/core/data/global.cxx:355
> #1 0x00007f4906f5e968 in ScAutoFormat::ScAutoFormat (this=0x335c510, nLim=4, nDel=4, bDup=0 '\000') at sc/source/core/tool/autoform.cxx:898
> #2 0x00007f4906ecbe93 in ScGlobal::GetAutoFormat () at sc/source/core/data/global.cxx:304
> #3 0x00007f490749b799 in ScAutoFormatObj::~ScAutoFormatObj (this=0x7f48fd089a10) at sc/source/ui/unoobj/afmtuno.cxx:441
> #4 0x00007f490749b880 in ScAutoFormatObj::~ScAutoFormatObj (this=0x7f48fd089a10) at sc/source/ui/unoobj/afmtuno.cxx:447
> #5 0x00007f492e87acd1 in cppu::OWeakObject::release (this=0x7f48fd089a10) at cppuhelper/source/weak.cxx:213
> [...]
> #40 0x00007f491736fdcc in binaryurp::Bridge::terminate (this=0x7f491538f1d0) at binaryurp/source/bridge.cxx:271
> [...]
> #55 0x00007f492f7d2377 in desktop::Desktop::DeregisterServices (this=0x7ffff56496c0) at desktop/source/app/appinit.cxx:378
> #56 0x00007f492f7ba4ee in desktop::Desktop::doShutdown (this=0x7ffff56496c0) at desktop/source/app/app.cxx:1875
> #57 0x00007f492f7b9a9d in desktop::Desktop::Main (this=0x7ffff56496c0) at desktop/source/app/app.cxx:1845
> #58 0x00007f492af3c6de in ImplSVMain () at vcl/source/app/svmain.cxx:178
> #59 0x00007f492af3c824 in SVMain () at vcl/source/app/svmain.cxx:215
> #60 0x00007f492f7f438a in soffice_main () at desktop/source/app/sofficemain.cxx:67
> #61 0x0000000000400734 in sal_main () at desktop/source/app/main.c:34
> #62 0x0000000000400719 in main (argc=9, argv=0x7ffff5649878) at desktop/source/app/main.c:33
ScGlobal::ppRscString has apparently already been cleared at this point.
But looking deeper down the stack, there is no real reason to create a
fresh ScAutoFormat at all -- ~ScAutoFormatObj does
ScAutoFormat* pFormats = ScGlobal::GetAutoFormat();
if ( pFormats && pFormats->IsSaveLater() )
but the freshly created ScAutoFormat (from ScGlobal::GetAutoFormat) has
bSaveLater set to false, so the condition would correctly already
evaluate to false if ScGlobal::GetAutoFormat would return null here. It
is also curious that pFormats is checked for null here, even though
ScGlobal::GetAutoFormat never returns null.
With that insight, I inspected all the other calls to
ScGlobal::GetAutoFormat, and many (but not all) of them too would only
dereference the returned ScAutoFormat pointer after checking it for
non-nullness. So I came up with the attached patch, introducing
ScGlobal::GetOrCreateAutoFormat for the few cases that apparently
require a non-null ScAutoFormat, and letting ScGlobal::GetAutoFormat
return null in case ScGlobal::pAutoFormat is null.
However, I had to fix one of the non-null--checking cases to use
GetOrCreateAutoFormat, as otherwise
sc/CppunitTest_sc_tableautoformatfield would fail with unexpected
IndexOutOfBoundsExceptions.
So, I would appreciate it if some Calc aficionado could look at the
patch, whether it actually makes any sense. It would be crucial to look
at the remaining calls to ScGlobal::GetAutoFormat, in
sc/source/core/data/table4.cxx
sc/source/ui/docshell/docfunc.cxx
sc/source/ui/unoobj/afmtuno.cxx
sc/source/ui/unoobj/cellsuno.cxx
and check whether not creating a fresh ScAutoFormat is OK for them, or
whether they should be calls to GetOrCreateAutoFormat after all (and the
confusing null-checks removed).
Stephan
More information about the LibreOffice
mailing list