Bug 34465 - get rid of all calls to virtual const SfxPoolItem* Put( const SfxPoolItem&, USHORT nWhich)

[Maciej Rumianowski]

Hi Bjoern,

2012/11/30 Bjoern Michaelsen <bjoern.michaelsen at canonical.com>

> Yes, there are quite a few evil cornercases lurking there. Before changing
> those conditionals, I think it is a good idea to just add assertions that
> make
> sure these assumptions are kept consistent all the time. A start would be
> to
> check that when an item is put in the set, to check:
> - If it has a non-zero Which-Id
> - and is not a SfxVoidItem
> any client code that does something different (even explicitly putting a
> SfxVoidItem), is abusing the API and should be fixed by making it call
> DisableItem instead). Once you are sure that is cleaned up, one can
> carefully
> go on about changing the conditions. Some care is still needed as in
> theory,
> someone might have e.g. put a non-zero, non-SfxVoidItem and later set the
> WhichId on the item to 0.
>

after a break I found sometime to hack :)
There is one place where SfxVoidItem is used instead of DisableItem
function.:
http://opengrok.libreoffice.org/xref/core/editeng/source/editeng/eerdll.cxx#150

> SfxPoolItem** GlobalEditData::GetDefItems()
> {
>     if ( !ppDefItems )
>     {
>         ppDefItems = new SfxPoolItem*[EDITITEMCOUNT];
>
>         // Paragraph attributes:
>         SvxNumRule aTmpNumRule( 0, 0, sal_False );
>
>         ppDefItems[0]  = new SvxFrameDirectionItem( FRMDIR_HORI_LEFT_TOP,
> EE_PARA_WRITINGDIR );
>         ppDefItems[1]  = new SvXMLAttrContainerItem( EE_PARA_XMLATTRIBS );
>         ppDefItems[2]  = new SfxBoolItem( EE_PARA_HANGINGPUNCTUATION,
> sal_False );
>         ppDefItems[3]  = new SfxBoolItem( EE_PARA_FORBIDDENRULES, sal_True
> );
> ...
>         ppDefItems[17] = new SvxVerJustifyItem( SVX_VER_JUSTIFY_STANDARD,
> EE_PARA_VER_JUST );
>
>         // Character attributes:
>         ppDefItems[18] = new SvxColorItem( Color( COL_AUTO ),
> EE_CHAR_COLOR );
>         ppDefItems[19] = new SvxFontItem( EE_CHAR_FONTINFO );
>  ...

        ppDefItems[38] = new SvxFontHeightItem( 240, 100,
> EE_CHAR_FONTHEIGHT_CTL );
>         ppDefItems[39] = new SvxWeightItem( WEIGHT_NORMAL,
> EE_CHAR_WEIGHT_CJK );
>         ppDefItems[40] = new SvxWeightItem( WEIGHT_NORMAL,
> EE_CHAR_WEIGHT_CTL );
>         ppDefItems[41] = new SvxPostureItem( ITALIC_NONE,
> EE_CHAR_ITALIC_CJK );
>         ppDefItems[42] = new SvxPostureItem( ITALIC_NONE,
> EE_CHAR_ITALIC_CTL );
>         ppDefItems[43] = new SvxEmphasisMarkItem( EMPHASISMARK_NONE,
> EE_CHAR_EMPHASISMARK );
>         ppDefItems[44] = new SvxCharReliefItem( RELIEF_NONE,
> EE_CHAR_RELIEF );
>         ppDefItems[45] = new SfxVoidItem( EE_CHAR_RUBI_DUMMY );
>         ppDefItems[46] = new SvXMLAttrContainerItem( EE_CHAR_XMLATTRIBS );
>         ppDefItems[47] = new SvxOverlineItem( UNDERLINE_NONE,
> EE_CHAR_OVERLINE );
>
>         // Features
>         ppDefItems[48] = new SfxVoidItem( EE_FEATURE_TAB );
>         ppDefItems[49] = new SfxVoidItem( EE_FEATURE_LINEBR );
>         ppDefItems[50] = new SvxCharSetColorItem( Color( COL_RED ),
> RTL_TEXTENCODING_DONTKNOW, EE_FEATURE_NOTCONV );
>         ppDefItems[51] = new SvxFieldItem( SvxFieldData(),
> EE_FEATURE_FIELD );
>
>         DBG_ASSERT( EDITITEMCOUNT == 52, "ITEMCOUNT geaendert, DefItems
> nicht angepasst!" );
>
>         // Init DefFonts:
>         GetDefaultFonts( *(SvxFontItem*)ppDefItems[EE_CHAR_FONTINFO -
> EE_ITEMS_START],
>                          *(SvxFontItem*)ppDefItems[EE_CHAR_FONTINFO_CJK -
> EE_ITEMS_START],
>                          *(SvxFontItem*)ppDefItems[EE_CHAR_FONTINFO_CTL -
> EE_ITEMS_START] );
>     }
>
>     return ppDefItems;
> }
>
What about this? Changing to SvxColorItem crashes
CppunitTest_sw_subsequent_odfexport on
Program received signal SIGSEGV, Segmentation fault.
0x45c4f134 in CharAttribList::InsertAttrib (this=0x8676b60, pAttrib=0x0) at
/home/maciej/programowanie/libreoffice/libo-working/editeng/source/editeng/editdoc.cxx:2661
2661        const sal_uInt16 nStart = pAttrib->GetStart(); // may be better
for Comp.Opt.
#0  0x45c4f134 in CharAttribList::InsertAttrib (this=0x8676b60,
pAttrib=0x0) at
/home/maciej/programowanie/libreoffice/libo-working/editeng/source/editeng/editdoc.cxx:2661
#1  0x45c4e7d3 in EditDoc::InsertAttrib (this=0x86766a0, rPoolItem=...,
pNode=0x8676b40, nStart=0, nEnd=0) at
/home/maciej/programowanie/libreoffice/libo-working/editeng/source/editeng/editdoc.cxx:2473
#2  0x45c4e9fb in EditDoc::InsertAttrib (this=0x86766a0, pNode=0x8676b40,
nStart=0, nEnd=0, rPoolItem=...) at
/home/maciej/programowanie/libreoffice/libo-working/editeng/source/editeng/editdoc.cxx:2515
#3  0x45d07e44 in ImpEditEngine::SetAttribs (this=0x8676630, aSel=...,
rSet=..., nSpecial=1 '\001') at
/home/maciej/programowanie/libreoffice/libo-working/editeng/source/editeng/impedit5.cxx:551

because based on ItemState it should be disabled and never go beyond #3.
This shows that Item EE_CHAR_RUBI_DUMMY (4036) is by default disabled, and
never explicitly with DisableItem function.

Thanks in advance for any help/advice

Best Regards,
    Maciek
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/libreoffice/attachments/20130220/730eb0b5/attachment.html>