Review Request 110139: Small bugfixes. Fixes clang warnings.

[Matěj Laitl]

> On April 24, 2013, 12:47 p.m., Myriam Schweingruber wrote:
> > Which exact bugs does that fix? are those reported on http://bugs.kde.org? Then please add the "BUG: <bugnumber>" handler and "FIXED-IN: 2.8" handler to it as well :)
> 
> Konrad Zemek wrote:
>     None of these bugs are reported in bugs.kde.org (at least I couldn't find any matching report).
>     In the order shown by reviewboard's diff, the bugs fixed are following:
>     
>     1. Introduced with https://bugs.kde.org/show_bug.cgi?id=305203 : !state() == EditingState is always evaluating to false, so instead of stopping 'return' key from adding tracks to the playlist while in edit mode, this fix accidently stopped the key from adding tracks in all modes.
>     
>     2. Obvious error when we're filling a struct with 0's, and instead of using the size of struct itself, we're using the size of pointer type.
>     
>     3 & 4. When an integer (enum in this case) is added to "string", it is actually evaluated as if it was
>     const char *str = "string";
>     const char *result = str + value; // a.k.a. str[value];
>     That's surely not what we want to achieve here. By using QString::number, a QString is first constructed from the integer and then added to "string", so we get a valid string like we wanted.
>     
>     5, 6 & 7. These are just to silence compiler warnings about not all enums being uses in a switch; in these switches we're providing cases for all relevant enum values, so default: break tells the compiler that we're sure nothing else should be handled.

Righto righto, I think the confusion came from the sentence "A few legitimate, non critical bugs" - which may suggest that the bugs were reported to bugs.kde.org.

We don't normally require to be elaborate about small cleanups & fixes that are obvious from the diff - don't fear. :-)


- Matěj


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


On April 24, 2013, 12:25 a.m., Konrad Zemek wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/110139/
> -----------------------------------------------------------
> 
> (Updated April 24, 2013, 12:25 a.m.)
> 
> 
> Review request for Amarok.
> 
> 
> Description
> -------
> 
> Small bugfixes. Fixes clang warnings.
> 
> A few legitimate, non critical bugs. Other changes are adding default case for enum switches where we're sure that all relevant cases have been covered.
> 
> 
> Diffs
> -----
> 
>   src/browsers/playlistbrowser/PlaylistBrowserView.cpp fc7fe14a10ae0f4b25f9b0e0ccb85c246b82313d 
>   src/core-impl/collections/daap/daapreader/authentication/md5.c 089118ff8bcd20aecade257c31d56a0447fc04de 
>   src/core-impl/collections/db/sql/SqlQueryMaker.cpp fc5f6df29d9e22a97712a31b2aa71bc0e6424b40 
>   src/core-impl/collections/upnpcollection/UpnpQueryMaker.cpp 93e068124beae4ba4f294e476d696cc0196dd298 
>   src/dialogs/EditFilterDialog.cpp ac26497dcc5e5cf7cb9e2ea1a821dbce83276d34 
> 
> Diff: http://git.reviewboard.kde.org/r/110139/diff/
> 
> 
> Testing
> -------
> 
> Changed lines no longer generate warnings.
> 
> 
> Thanks,
> 
> Konrad Zemek
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/amarok-devel/attachments/20130425/9640d62b/attachment.html>