Re: Review for a lists patch

HI Marc,
            Here is a much more extensive patch that removes m_pParent
entirely. It works fine in all my tests.
Feel free to commit if it fixes your crash.

Cheers

Martin

On Sat, Nov 21, 2009 at 11:54 AM, Martin Sevior <msevior@gmail.com> wrote:
> On Sat, Nov 21, 2009 at 11:51 AM, Martin Sevior <msevior@gmail.com> wrote:
>> On Sat, Nov 21, 2009 at 2:46 AM, J.M. Maurer <uwog@uwog.net> wrote:
>>> Say you have some hypothetical list definitions like this:
>>>
>>> <lists>
>>>    <l id="1001"/>
>>>    <l id="1002" parentid="1001"/>
>>>    <l id="1003" parentid="1001"/>
>>> </lists>
>>>
>>> And also say you never ever use list 1001 in your document. This is
>>> silly of course, but in theory it could be valid. AbiWord itself will
>>> never write that out, but I actually came across generated documents
>>> like that :)
>>>
>>> Abiword will crash on this construct (valgrind trace attached):
>>>
>>> 1. PD_Document::updateDirtyLists will be called
>>> 2. It will see the first list is empty:
>>>
>>>       if(pAutoNum->isEmpty())
>>>
>>>   and it will delete it.
>>> 3. Then it will updat the other dirty lists a few lines below:
>>>
>>>       pAutoNum->update(0);
>>>
>>> => This will crash in fl_AutoNum::update() on the following line:
>>>
>>>       if (m_pParent && !m_pParent->isUpdating())
>>>
>>
>> A better solution would be to remove the m_pParent pointer and instead
>> record the
>> parent ID in fl_AutoNum. Call it m_iParentID.
>>
>> Then whenever need the parent  fl_AutoNum from the looking up the
>> parent ID in the Document.
>>
>> fl_AutoNum * pParent = m_pDoc->getListByID(m_iParentID);
>>
>> We could wrap all this into a const method
>>
>> fl_AutoNum::getParent(void) const
>> {
>>   return  m_pDoc->getListByID(m_iParentID);
>> }
>>
>
> Actually we already have m_iParentID and getParent methods. I'll check
> to see if we can get away with deleting m_pParent.
>
> Cheers
>
> Martin
>