[Libreoffice] odd ND_FOONODE bits was fdo#42147: fix crash

[Michael Stahl]

On 09/12/11 12:34, Caolán McNamara wrote:
> On Fri, 2011-12-09 at 09:44 +0400, Ivan Timofeev wrote:
>> 08.12.2011 23:36, Ivan Timofeev пишет:
>> This is not a fix for the mentioned bug, the proper fix was
>> http://cgit.freedesktop.org/libreoffice/core/commit/?id=3524727db0f3cfecf3a47046795c527808c10c3e
>> which is now backported to 3-4.
> 
> I wouldn't call my fix a great solution, more of a band-aid, but good to
> know it makes the crash go away.
> 
> Looking at your original patch anyway it looks right to me. Can't cast
> something whose NodeType is ND_CONTENTNODE which is a superset of the
> ND_TEXTNODE bits to a SwTxtNode seeing as a SwTxtNode inherits from
> SwCntntNode so not all SwCntntNodes are SwTxtNodes.

but the previous test was on ND_TEXTNODE, so the patch shouldn't change
the behaviour, it is however a nice stylistic cleanup

> It's all a little strange, i.e. 
> ND_TEXTNODE of 0x08 and a
> ND_CONTENTNODE of 0x38
> with
> inline sal_Bool SwNode::IsCntntNode() const
> {
>     return ND_CONTENTNODE & nNodeType  ? sal_True : sal_False;
> }
> 
> inline sal_Bool SwNode::IsTxtNode() const
> {
>     return ND_TEXTNODE == nNodeType  ? sal_True : sal_False;
> }
> 
> sees very odd. It suggests that IsCntntNode on a SwTxtNode is "false",
> even though a SwTxtNode isa SwCntntNode.

indeed; it also seems that SwNode::GetCntntNode only returns 0

d'oh!  no, we are just unable to read & correctly:
if _any_ of the bits in 0x38 is set in the nNodeType, then the &
evaluates to true; so it doesn't seem broken after all.

> Looking at...
> const BYTE ND_TEXTNODE          = 0x08;
> const BYTE ND_GRFNODE           = 0x10;
> const BYTE ND_OLENODE           = 0x20;
> const BYTE ND_CONTENTNODE       = 0x38;
> const BYTE ND_NOTXTNODE         = 0x30;
> and then http://docs.libreoffice.org/sw/html/classSwCntntNode.html
> this is equivalent to
> const BYTE ND_GRFNODE           = 0x10;
> const BYTE ND_OLENODE           = 0x20;
> const BYTE ND_NOTXTNODE         = ND_GRFNODE | ND_OLENODE;
> const BYTE ND_TEXTNODE          = 0x08;
> const BYTE ND_CONTENTNODE       = ND_NOTXTNODE | ND_TEXTNODE;
> this presumably is the derivation of the bits.
> 
> These bits seem inverted in the sense that the default bits for a
> baseclass are the superset of all possible derived class bits!?
> 
> On the other hand
> 
> const BYTE ND_STARTNODE         = 0x02;
> const BYTE ND_TABLENODE         = 0x06;
> const BYTE ND_SECTIONNODE       = 0x42;
> 
> is equivalent to 
> const BYTE ND_STARTNODE         = 0x02;
> const BYTE ND_TABLENODE         = 0x04 | ND_STARTNODE;
> const BYTE ND_SECTIONNODE       = 0x40 | ND_STARTNODE;
> 
> and the hierarchy there is
> http://docs.libreoffice.org/sw/html/classSwStartNode.html
> i.e. more derived class *add* more bits, which makes far more sense.

hmm.. would be nice if that were more consistent...

> caolanm->mstahl: Any insights here into why this is the way it is ?

well i can only speculate, but perhaps it all makes more sense after
consuming sufficient amounts of mind altering substances.

> Under what circumstances *can* IsCntntNode be true, I mean on a casual
> inspection do we have any bare SwCntntNodes in existance, as opposed to
> things that inherit from it and set a different NodeType

i don't believe we ever instantiate bare SwCntntNodes, only its subclasses.

the SwCntntNode also has other bizarre aspects like the whole SwIndexReg
and Len() thing that is only actually used by SwTxtNode...

perhaps many years ago there was another subclass of SwCntntNode, there
is a comment:

    // Returns count of elements of node content. Default is 1.
    // There are differences between text node and formula node.
    virtual xub_StrLen Len() const;