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

[Kevin Krammer]

> On June 4, 2012, 7:22 p.m., Kevin Krammer wrote:
> > kalarm/lib/spinbox.cpp, line 533
> > <http://git.reviewboard.kde.org/r/105149/diff/1/?file=66301#file66301line533>
> >
> >     space after ( and before ), see above
> 
> David Jarvie wrote:
>     KAlarm does not use spaces inside brackets, so the proposed patch is correct.

Ah, my mistake. didn't see that this was a KAlarm file, sorry


> 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?
> 
> Jaime Torres Amate wrote:
>     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.
>

That's what I meant when I wrote that it looks plausible.
I just thought that it might be safer to check back with the author who wrote it.


- Kevin


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