| Summary: | Review: utilities/cameragui/mtqueue.h use bad locking style | ||
|---|---|---|---|
| Product: | [Applications] digikam | Reporter: | lan |
| Component: | Import-MainView | Assignee: | Digikam Developers <digikam-bugs-null> |
| Status: | RESOLVED INTENTIONAL | ||
| Severity: | wishlist | ||
| Priority: | NOR | ||
| Version: | 0.7.0 | ||
| Target Milestone: | --- | ||
| Platform: | unspecified | ||
| OS: | Linux | ||
| Latest Commit: | Version Fixed In: | ||
| Attachments: | Better locking style | ||
Created attachment 8121 [details]
Better locking style
Forgot to add: this patch also remove lock() and unlock() methods in mtqueue and aslo make method head() without argument. These methods are not used at now. can you give me an example of a case where the mutex will not be unlocked? And digikam doesn't use exceptions. QMutexLocker is just a convenience function for locking and unlocking mutex, so that the programmer doesn't have to remember to unlock the mutex for each scenario. You are right, where are can be any exceptions. That is why original code is working right. I just thought it would be better to make code nicer. not using qmutexlocker is not a sin :) |
Version: 0.7-beta1 (using KDE 3.2.1, SuSE) Compiler: gcc version 3.3.3 (SuSE Linux) OS: Linux (i686) release 2.6.5-7.111-default It use something like this: { mutex.lock(); do_something(); mutex.unloack(); } in case of exception or other error lock will not be unlocked. Qt suggest better style by using QMutexLocker: { QMutexLocker locker(&mutex); do_something(); } locker lock mutex in constructor and unlock it in the destructor and as far as it created on the stack it WILL be unlocked. Patch attached.