Re [2]: CVS: commit tomas_f abi/src/text/fmt/xp fp_Page.cpp

Just disregard my previous email -- I was confusing fl_HdrFtrShadow with
fp_ShadowContainer. The original patch was I think correct and benign,
but I will run more tests before putting it back.

Tomas

Tomas Frydrych wrote:
>
>
> msevior@physics.unimelb.edu.au wrote:
>
>>> --- fp_Page.cpp 7 Jul 2005 14:24:49 -0000 1.184
>>> +++ fp_Page.cpp 18 Aug 2005 18:35:04 -0000 1.185
>>> @@ -84,6 +84,9 @@
>>> m_pOwner = NULL;
>>> pDSL->deleteOwnedPage(this);
>>> }
>>> +
>>> + delete m_pFooter;
>>> + delete m_pHeader;
>>> }
>>
>>
>> Eeek! If this code works 95% of the time then I have left some bad
>> code in
>> place for ages.
>>
>> I'm nervous about this patch though.
>
>
> I have had another look at this and there is indeed a pontential for
> screw up and the memory leak is probably the lesser of the two evils at
> this stage so I will revert it (and consider a better way of fixing this).
>
> The basic problem is that in the hdr/ftr code it is not at all clear who
> owns what. The fp_HdrFtrShadow object is effectively owned by fp_Page
> (it is allocated and stored there, and it is specific to that instance
> of it); but the shadow is also cached in fl_HdrFtrSectionLayout's
> m_vecPages, and ~fl_HdrFtrSectionLayout is relied on for its
> destruction. This is not great from the design point of view, because
> you are bound to end up with dangling pointers and/or memory leaks if
> the code is later modified by someone like me who is not familiar into
> the last detail with the intricacies of the relationship between the
> different classes.
>
> Basically, ~fl_HdrFtrSectionLayout has no business to be deleting
> pointer that are owned by fp_Page; it should remove the pointers it
> caches, and tell fp_Page to delete the object (so that it can NULL the
> member). (This is also the reason why the 'delete this' construct is so
> bad; it means that a dangling pointer to 'this' is left somewhere;
> objects should only be deleted by their owners.)
>
> Tomas
>
>
> ___________________________________________________________ To help you
> stay safe and secure online, we've developed the all new Yahoo! Security
> Centre. http://uk.security.yahoo.com
>
>

        
        
                
___________________________________________________________
Yahoo! Messenger - NEW crystal clear PC to PC calling worldwide with
voicemail http://uk.messenger.yahoo.com

                
___________________________________________________________
How much free photo storage do you get? Store your holiday
snaps for FREE with Yahoo! Photos http://uk.photos.yahoo.com
Received on Fri Aug 19 11:26:43 2005