On Fri, Mar 15, 2013 at 8:34 AM, Simeon Bird <[email protected]> wrote:
> Hi, > > I read through the patches - 0002, 0004 and 0005 are good. For 0001, > can you not instead of this: > > lock.unlock(); > QMutexLocker rmlock(&m_rm->mutex); // for updateKickOffLists, but > must be locked first > lock.relock(); // we must respect the lock ordering! > > const QString newNaoIdentifier = > m_cache.value(NAO::identifier()).toString(); > const QUrl newNieUrl = m_cache.value(NIE::url()).toUrl(); > updateIdentifierLists( oldNaoIdentifier, newNaoIdentifier ); > > do this: > > const QString newNaoIdentifier = > m_cache.value(NAO::identifier()).toString(); > const QUrl newNieUrl = m_cache.value(NIE::url()).toUrl(); > > lock.unlock(); > QMutexLocker rmlock(&m_rm->mutex); // for updateKickOffLists, but > must be locked first > > updateIdentifierLists( oldNaoIdentifier, newNaoIdentifier ); > > No rule says you can't take the mutexes independently, only that if > you take them both you must take the rmmutex first, no? > > For patch 3, how about the attached two patches instead? > I like your patch about making init a readWrite one, but not about the resourceStart/stop one. I cannot comment on the issue above, I'll need to look at the code more carefully. Also, I don't really understand your - ResourceData: Change updateKickOffLists to take three arguments, the uri, old and new values, patch. I think I'll try to look at it again, a little later. > I pushed a feature branch: feature/resourcemanagercleanup with both > these patch series in them. It's on top of KDE/4.10, and your patches > are on top of mine. Does it make sense to you? Vishesh, what do you > think? > > Thanks > Simeon > > _______________________________________________ > Nepomuk mailing list > [email protected] > https://mail.kde.org/mailman/listinfo/nepomuk > > -- Vishesh Handa
_______________________________________________ Nepomuk mailing list [email protected] https://mail.kde.org/mailman/listinfo/nepomuk
