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

On Fri, 2011-12-09 at 13:31 +0100, Michael Stahl wrote:
> On 09/12/11 12:34, Caolán McNamara wrote:
> > 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.
>
> The previous test was on ND_TEXTNODE, so the patch shouldn't change
> the behaviour, it is however a nice stylistic cleanup

Just to be clear, given my later confusion, in the "good case" where it
is a SwTxtNode there is no change, while if it was somehow a underived
SwCntntNode there would be a desirable change in behaviour.

const sal_uInt8 ND_CONTENTNODE  = 0x38;
const sal_uInt8 ND_TEXTNODE     = 0x08;

inline sal_Bool SwNode::IsTxtNode() const
{
    return ND_TEXTNODE == nNodeType  ? sal_True : sal_False;
}

nNdTyp = ND_CONTENTNODE; //evil

- if( ND_TEXTNODE & nNdTyp )
^^^ would be 8, i.e. true
+ if( rNd.IsTxtNode() )
^^^ would be false

nNdTyp = ND_TEXTNODE; //good

- if( ND_TEXTNODE & nNdTyp )
^^^ would be 8, i.e. true
+ if( rNd.IsTxtNode() )
^^^ would be true


> > 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;
> > }
>
> 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.

*slap forehead*, sigh, indeed. SwTxtNodes are SwCntntNodes alright wrt 
ND_CONTENTNODE & ND_TEXTNODE.

Ah well, its probably worth munging in the derivation of the bits as |
of the super-class bits anyway so its a little more clear. Should ditch
this in favour of real rtti if there's no gotchass.

C.