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.
_______________________________________________
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice
