Review Request: one memory leak, 7 exceptions and two comparations.

Thorsten Zachmann t.zachmann at zagge.de
Thu Jun 23 05:20:40 BST 2011


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

Ship it!


Thanks for the patch. I have added some comments. Please commit after incorporating the changes.


filters/karbon/applixgraphics/applixgraphicimport.cc
<http://git.reviewboard.kde.org/r/101730/#comment3338>

    Right. Maybe you can add a TODO to remove the new/delete alltogether by just having one for loop to write out the data, then no array is needed for storing the data at all.



filters/kpresenter/powerpoint/ParsedPresentation.cpp
<http://git.reviewboard.kde.org/r/101730/#comment3330>

    Please make the variable also const.



filters/kpresenter/powerpoint/ParsedPresentation.cpp
<http://git.reviewboard.kde.org/r/101730/#comment3331>

    Please make the variable also const.



filters/kpresenter/powerpoint/ParsedPresentation.cpp
<http://git.reviewboard.kde.org/r/101730/#comment3332>

    Please make the variable also const.



filters/libmso/ODrawToOdf.cpp
<http://git.reviewboard.kde.org/r/101730/#comment3333>

    Please make the variable also const.



filters/libmso/ODrawToOdf.cpp
<http://git.reviewboard.kde.org/r/101730/#comment3334>

    Please make the variable also const.



filters/libmso/ODrawToOdf.cpp
<http://git.reviewboard.kde.org/r/101730/#comment3335>

    Please make the variable also const.



filters/libmso/ODrawToOdf.cpp
<http://git.reviewboard.kde.org/r/101730/#comment3336>

    Please make the variable also const.



filters/tables/excel/sidewinder/format.h
<http://git.reviewboard.kde.org/r/101730/#comment3337>

    Good catch


- Thorsten


On June 22, 2011, 9:46 p.m., Jaime Torres Amate wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/101730/
> -----------------------------------------------------------
> 
> (Updated June 22, 2011, 9:46 p.m.)
> 
> 
> Review request for Calligra.
> 
> 
> Summary
> -------
> 
> The two comparations are obvious.
> The memory leak, I think Qt will not delete them.
> The exceptions, following C++ style guides, the variable is preferred by reference.
> 
> 
> Diffs
> -----
> 
>   filters/karbon/applixgraphics/applixgraphicimport.cc e8407fd 
>   filters/kpresenter/powerpoint/ParsedPresentation.cpp 0f375b9 
>   filters/libmso/ODrawToOdf.cpp 6551a62 
>   filters/tables/excel/sidewinder/format.h e744a1c 
> 
> Diff: http://git.reviewboard.kde.org/r/101730/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Jaime Torres
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/calligra-devel/attachments/20110623/1e5b1a5c/attachment.htm>


More information about the calligra-devel mailing list