Re: msevior - r30086 - abiword/trunk/src/text/ptbl/xp

On Thu, Aug 4, 2011 at 3:49 AM, <cvs@abisource.com> wrote:
>
> Author: msevior
> Date: 2011-08-04 08:49:22 +0200 (Thu, 04 Aug 2011)
> New Revision: 30086
>
> Modified:
>   abiword/trunk/src/text/ptbl/xp/pd_Document.cpp
>   abiword/trunk/src/text/ptbl/xp/pf_Fragments.cpp
> Log:
>
> Robustness fixes.
>
>
>
> Modified: abiword/trunk/src/text/ptbl/xp/pd_Document.cpp
> ===================================================================
> --- abiword/trunk/src/text/ptbl/xp/pd_Document.cpp      2011-08-02 14:32:57 UTC (rev 30085)
> +++ abiword/trunk/src/text/ptbl/xp/pd_Document.cpp      2011-08-04 06:49:22 UTC (rev 30086)
> @@ -1664,6 +1664,7 @@
>        for(i=0; i< m_vecSuspectFrags.getItemCount(); i++)
>        {
>                pf = m_vecSuspectFrags.getNthItem(i);
> +               UT_DEBUGMSG(("Suspect frag %d pointer %p \n",i,pf));
>                if(pf->getType() == pf_Frag::PFT_Strux)
>                {
>                        pfs = static_cast<pf_Frag_Strux *>(pf);
>
> Modified: abiword/trunk/src/text/ptbl/xp/pf_Fragments.cpp
> ===================================================================
> --- abiword/trunk/src/text/ptbl/xp/pf_Fragments.cpp     2011-08-02 14:32:57 UTC (rev 30085)
> +++ abiword/trunk/src/text/ptbl/xp/pf_Fragments.cpp     2011-08-04 06:49:22 UTC (rev 30086)
> @@ -852,18 +852,18 @@
>  {
>        UT_ASSERT(pn != NULL);
>

Martin,

I didn't understand your changes here.
IIUC, UT_ASSERT will ensure that pn isn't NULL, so, why check for pn in all ifs?

> -       if (pn != m_pLeaf)
> +       if (pn && pn != m_pLeaf)
>        {

Here, to avoid some problem, would be cool check for pn->left &&
pn->left != m_pLeaf, and the same below, with pn->right && pn->right
!= m_pLeaf

> -               if (pn->left != m_pLeaf)
> +               if (pn && pn->left != m_pLeaf)
>                {
>                        pn = pn->left;
>
> -                       while(pn->right != m_pLeaf)
> +                       while(pn && pn->right != m_pLeaf)
>                                pn = pn->right;
>                }
>                else
>                {
> -                       while(pn->parent)

Here, pn could be changed into this while?

> +                       while(pn && pn->parent)
>                        {
>                                if (pn->parent->right == pn)
>                                        return pn->parent;
> @@ -888,24 +888,35 @@
>  pf_Fragments::_next(const Node* pn) const
>  {
>        UT_ASSERT(pn != NULL);

Same commented above

> -
> -       if (pn != m_pLeaf)
> +       if(pn == 0)
> +         return 0;
> +       if (pn && pn != m_pLeaf)
>        {
> -               if (pn->right != m_pLeaf)
> +               if (pn && pn->right != m_pLeaf)
>                {
>                        pn = pn->right;
>
> -                       while(pn->left != m_pLeaf)
> +                       while(pn && pn->left != m_pLeaf)
> +                       {
>                                pn = pn->left;
> +                       }
>                }
>                else
>                {
> -                       while(pn->parent)
> +                       while(pn && pn->parent)
>                        {
> -                               if (pn->parent->left == pn)
> +                               if (pn && pn->parent->left == pn)
> +                               {
>                                        return pn->parent;
> +                               }
> +                               else if(pn)
> +                               {
> +                                       pn = pn->parent;
> +                               }
>                                else
> -                                       pn = pn->parent;
> +                               {
> +                                      return 0;
> +                               }
>                        }
>
>                        return 0;
>
> -----------------------------------------------
> To unsubscribe from this list, send a message to
> abisource-cvs-commit-request@abisource.com with the word
> unsubscribe in the message body.
>

Best Regards,

-- 
Fabiano Fidêncio