[Kde-pim] Review Request: fix for some cppcheck and clang errors

Jaime Torres Amate jtamate at gmail.com
Tue Jun 5 08:34:54 BST 2012



> On June 4, 2012, 7:22 p.m., Kevin Krammer wrote:
> > kjots/kjotsbrowser.cpp, line 61
> > <http://git.reviewboard.kde.org/r/105149/diff/1/?file=66302#file66302line61>
> >
> >     this looks plausible but it changes the condition. is there any bug associated with that?

Are you sure this is not the real intention of the original coder?
if a and (b=c or b=d)      
Now the compiler can choose to do
if (a and b=c) or b=d, that looks wrong to me.


> On June 4, 2012, 7:22 p.m., Kevin Krammer wrote:
> > kleopatra/utils/gnupg-helper.cpp, line 155
> > <http://git.reviewboard.kde.org/r/105149/diff/1/?file=66304#file66304line155>
> >
> >     also plausible but again changes the condition

again.
I'm sure the original coder intention was to check for \n or \r while end!=0
I do not think the intention was to check for ( end && line[end-1]=='\n' ) || (line[end-1]=='\r').
 


- Jaime Torres


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


On June 4, 2012, 6:28 p.m., Jaime Torres Amate wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/105149/
> -----------------------------------------------------------
> 
> (Updated June 4, 2012, 6:28 p.m.)
> 
> 
> Review request for KDEPIM.
> 
> 
> Description
> -------
> 
> 1. change initialization order to follow variable declaration order
> 2. initialize the variable
> 3. do nothing if m is null.
> 4. adding a const bool to a const char* does not appends the value. Changed to QString with arg
> 5. add parenthesis to clarify ? expressions priority
> 6. add parenthesis to clarify ? expressions priority
> 7. add parenthesis to clarify || vs && priority
> 8. i++ to ++i (little faster)
> 9. add parenthesis to clarify || vs && priority
> 10. i++ to ++i (little faster)
> 11. avoid using tree if tree is null
> 12. avoid using extern "C" if the function return a class, incompatible C linkage (the comments are already removed)
> 
> 
> Diffs
> -----
> 
>   akonadi_next/kreparentingproxymodel.cpp 411e3fc 
>   akregator/src/importfeedlistcommand.cpp 117237b 
>   akregator/src/subscriptionlistview.cpp de41757 
>   calendarsupport/next/incidencechanger2.cpp dd9fbd1 
>   kaddressbook/printing/mikesstyle.cpp 1ac65cf 
>   kalarm/lib/spinbox.cpp ab2e9ec 
>   kjots/kjotsbrowser.cpp 3970cfc 
>   kleopatra/smartcard/readerstatus.cpp 3b9c282 
>   kleopatra/utils/gnupg-helper.cpp 02518db 
>   kmail/undostack.cpp d981769 
>   knode/foldertreewidget.cpp c0d1a92 
>   plugins/ktexteditor/ktexteditorkabcbridge.cpp 050d478 
> 
> Diff: http://git.reviewboard.kde.org/r/105149/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Jaime Torres Amate
> 
>

_______________________________________________
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/



More information about the kde-pim mailing list