[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