Review Request: Fix crash when closing Amarok with running CoverFetcher

[Ralf Engels]

> On Aug. 9, 2012, 10:45 a.m., Matěj Laitl wrote:
> > Looks good, just a few small things. Could you also please describe the steps that lead to the crash in the commit msg?

Really? It's in the bug report pretty clearly. No need to copy it in my opinion.


> On Aug. 9, 2012, 10:45 a.m., Matěj Laitl wrote:
> > src/statusbar/CompoundProgressBar.h, line 32
> > <http://git.reviewboard.kde.org/r/105942/diff/1/?file=76719#file76719line32>
> >
> >     Please document the fact that this class is now completely thread-safe. (or, if not, document that it is reentrant and some methods are thread-safe, then document every thread-safe method)

I have no idea if it's now completely thread-save. I just fixed the most obvious problems.


> On Aug. 9, 2012, 10:45 a.m., Matěj Laitl wrote:
> > src/statusbar/CompoundProgressBar.cpp, line 46
> > <http://git.reviewboard.kde.org/r/105942/diff/1/?file=76720#file76720line46>
> >
> >     Nitpicky: I would personally strip all the added newlines

Actually I think that newlines are a good device to structure code.
I use it to split different functional blocks inside a function.

Often I use them to split up between the initialization, the actual function and the cleaning up inside a function.
That's why there is a newline between the Locker and the rest of the code.


- Ralf


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


On Aug. 9, 2012, 10:36 a.m., Ralf Engels wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/105942/
> -----------------------------------------------------------
> 
> (Updated Aug. 9, 2012, 10:36 a.m.)
> 
> 
> Review request for Amarok.
> 
> 
> Description
> -------
> 
> childBarFinished was called with invalid m_progressDetailsWidget
> However also the m_progressMap needs protection. The CompoundProgressBar needs to be thread safe.
> 
> 
> This addresses bug 292553.
>     https://bugs.kde.org/show_bug.cgi?id=292553
> 
> 
> Diffs
> -----
> 
>   src/statusbar/CompoundProgressBar.h baaab94 
>   src/statusbar/CompoundProgressBar.cpp 371a534 
> 
> Diff: http://git.reviewboard.kde.org/r/105942/diff/
> 
> 
> Testing
> -------
> 
> Change is verified by me to fix the problem.
> 
> 
> Thanks,
> 
> Ralf Engels
> 
>

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