Re: Review for a lists patch

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
Received on Sat Nov 21 01:59:06 2009