[Kde-pim] Review Request: KMime::Content -- provide clean way of removing sub-contents

[Thomas McGuire]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.kde.org/r/963/#review1523
-----------------------------------------------------------

Ship it!


Looks good, please commit.


/trunk/KDE/kdepimlibs/kmime/kmime_content.h
<http://reviewboard.kde.org/r/963/#comment952>

    The fact that this is different than calling removeContents() on all contents should be mentioned in the API dox. So the docs should mention that it is not possible that this is converted into a single-part content.


- Thomas


On 2009-07-09 03:24:57, Constantin Berzan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/963/
> -----------------------------------------------------------
> 
> (Updated 2009-07-09 03:24:57)
> 
> 
> Review request for KDE PIM.
> 
> 
> Summary
> -------
> 
> If a multi-part content has two subcontents, and one is removed, it automatically transforms itself into a single-part content.  This is a problem in a scenario like the following:
> 
> // stuff has 2 sub-contents.
> Content::List contents = stuff->contents();
> otherStuff->addContent( contents[0] );
> otherStuff->addContent( contents[1] ); // crash
> 
> The crash happens because the first addContent calls stuff->removeContent( contents[0] ), and stuff has only one subcontent left (contents[1]), so it turns into a single-part content, deleting contents[1] and making that a dangling pointer.  This is more or less what happens in the kmime_message_test (which crashes, and Valgrind clearly shows access-after-delete nastiness).
> 
> The solution proposed here adds a KMime::Content::clearContents( bool del ) function to allow detaching of subcontents from a parent.  I guess another solution would be to add another argument to removeContent( Content *c, bool del = false, bool autoSingle = true );  I'm not sure which is best.
> 
> This problem was introduced in revision 980456, when contents started being auto-removed from their old parent when re-parented.  (But I guess it existed in a more subtle form even before.)
> 
> 
> Diffs
> -----
> 
>   /trunk/KDE/kdepimlibs/kmime/kmime_content.h 993669 
>   /trunk/KDE/kdepimlibs/kmime/kmime_content.cpp 993669 
>   /trunk/KDE/kdepimlibs/kmime/tests/kmime_message_test.cpp 993669 
> 
> Diff: http://reviewboard.kde.org/r/963/diff
> 
> 
> Testing
> -------
> 
> kmime_message_test now passes without memory errors.
> 
> 
> Thanks,
> 
> Constantin
> 
>

_______________________________________________
KDE PIM mailing list kde-pim at kde.org
https://mail.kde.org/mailman/listinfo/kde-pim
KDE PIM home page at http://pim.kde.org/