Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

fix: remove handle without fail handles greater than the removed one.

  • Loading branch information...
commit e7336d80f7c399bcfeba2bee706fa6c219fde684 1 parent d446c0a
ulion authored
43 xbmc/filesystem/PluginDirectory.cpp
View
@@ -73,7 +73,23 @@ void CPluginDirectory::removeHandle(int handle)
{
CSingleLock lock(m_handleLock);
if (handle >= 0 && handle < (int)globalHandles.size())
- globalHandles.erase(globalHandles.begin() + handle);
+ {
+ globalHandles[handle] = NULL;
+ while (handle >= 0 && handle + 1 == (int)globalHandles.size() && NULL == globalHandles[handle])
+ {
+ globalHandles.erase(globalHandles.begin() + handle);
+ --handle;
+ }
+ }
+}
+
+// easy method to get dir object from handle.
+// lock only in the function maybe not strong enough, make sure lock before call it.
+CPluginDirectory *CPluginDirectory::dirFromHandle(int handle)
+{
+ if (handle >= 0 && handle < (int)globalHandles.size())
+ return globalHandles[handle];
+ return NULL;
}
bool CPluginDirectory::StartScript(const CStdString& strPath, bool retrievingDir)
@@ -161,13 +177,13 @@ bool CPluginDirectory::GetPluginResult(const CStdString& strPath, CFileItem &res
bool CPluginDirectory::AddItem(int handle, const CFileItem *item, int totalItems)
{
CSingleLock lock(m_handleLock);
- if (handle < 0 || handle >= (int)globalHandles.size())
+ CPluginDirectory *dir = dirFromHandle(handle);
+ if (NULL == dir)
{
CLog::Log(LOGERROR, " %s - called with an invalid handle.", __FUNCTION__);
return false;
}
- CPluginDirectory *dir = globalHandles[handle];
CFileItemPtr pItem(new CFileItem(*item));
dir->m_listItems->Add(pItem);
dir->m_totalItems = totalItems;
@@ -178,13 +194,13 @@ bool CPluginDirectory::AddItem(int handle, const CFileItem *item, int totalItems
bool CPluginDirectory::AddItems(int handle, const CFileItemList *items, int totalItems)
{
CSingleLock lock(m_handleLock);
- if (handle < 0 || handle >= (int)globalHandles.size())
+ CPluginDirectory *dir = dirFromHandle(handle);
+ if (NULL == dir)
{
CLog::Log(LOGERROR, " %s - called with an invalid handle.", __FUNCTION__);
return false;
}
- CPluginDirectory *dir = globalHandles[handle];
CFileItemList pItemList;
pItemList.Copy(*items);
dir->m_listItems->Append(pItemList);
@@ -196,12 +212,12 @@ bool CPluginDirectory::AddItems(int handle, const CFileItemList *items, int tota
void CPluginDirectory::EndOfDirectory(int handle, bool success, bool replaceListing, bool cacheToDisc)
{
CSingleLock lock(m_handleLock);
- if (handle < 0 || handle >= (int)globalHandles.size())
+ CPluginDirectory *dir = dirFromHandle(handle);
+ if (NULL == dir)
{
CLog::Log(LOGERROR, " %s - called with an invalid handle.", __FUNCTION__);
return;
}
- CPluginDirectory *dir = globalHandles[handle];
// set cache to disc
dir->m_listItems->SetCacheToDisc(cacheToDisc ? CFileItemList::CACHE_IF_SLOW : CFileItemList::CACHE_NEVER);
@@ -219,14 +235,13 @@ void CPluginDirectory::EndOfDirectory(int handle, bool success, bool replaceList
void CPluginDirectory::AddSortMethod(int handle, SORT_METHOD sortMethod, const CStdString &label2Mask)
{
CSingleLock lock(m_handleLock);
- if (handle < 0 || handle >= (int)globalHandles.size())
+ CPluginDirectory *dir = dirFromHandle(handle);
+ if (NULL == dir)
{
CLog::Log(LOGERROR, "%s - called with an invalid handle.", __FUNCTION__);
return;
}
- CPluginDirectory *dir = globalHandles[handle];
-
// TODO: Add all sort methods and fix which labels go on the right or left
switch(sortMethod)
{
@@ -556,12 +571,12 @@ bool CPluginDirectory::WaitOnScriptResult(const CStdString &scriptPath, int scri
void CPluginDirectory::SetResolvedUrl(int handle, bool success, const CFileItem *resultItem)
{
CSingleLock lock(m_handleLock);
- if (handle < 0 || handle >= (int)globalHandles.size())
+ CPluginDirectory *dir = dirFromHandle(handle);
+ if (NULL == dir)
{
CLog::Log(LOGERROR, " %s - called with an invalid handle.", __FUNCTION__);
return;
}
- CPluginDirectory* dir = globalHandles[handle];
dir->m_success = success;
*dir->m_fileResult = *resultItem;
@@ -613,13 +628,13 @@ void CPluginDirectory::SetContent(int handle, const CStdString &strContent)
void CPluginDirectory::SetProperty(int handle, const CStdString &strProperty, const CStdString &strValue)
{
CSingleLock lock(m_handleLock);
- if (handle < 0 || handle >= (int)globalHandles.size())
+ CPluginDirectory *dir = dirFromHandle(handle);
+ if (NULL == dir)
{
CLog::Log(LOGERROR, "%s called with an invalid handle.", __FUNCTION__);
return;
}
- CPluginDirectory *dir = globalHandles[handle];
if (strProperty == "fanart_image")
dir->m_listItems->SetArt("fanart", strValue);
else
1  xbmc/filesystem/PluginDirectory.h
View
@@ -71,6 +71,7 @@ class CPluginDirectory : public IDirectory
static std::vector<CPluginDirectory*> globalHandles;
static int getNewHandle(CPluginDirectory *cp);
static void removeHandle(int handle);
+ static CPluginDirectory *dirFromHandle(int handle);
static CCriticalSection m_handleLock;
CFileItemList* m_listItems;

4 comments on commit e7336d8

jmarshallnz

I suggest grabbing the first NULL handle in getNewHandle() as well (should one exist). That way you'll cleanup the first handles as they are done. I'd also lock in dirFromHandle() as otherwise it is likely to be called later without the lock.

I'd also add a comment to the block that's cleaning out the handles at the end of the vector so it's clear what the code is doing.

Looks good other than that - thanks!

ulion
Owner

the first thought I also want to use the first NULL value handle, but I found always use greatest handle for the new one will let debug more easy, and there's nothing to lose in that condition but more debug friendly, so I choose this plan.
about the lock, you are right, I will try update this PR.

jmarshallnz

That's fine - update the PR with the fix (just squash the fix into this commit and force push to your repo) to the locking and I'll take another look before pulling it in.

ulion
Owner

I didn't known the squash stuff before this, now I known how to co-work on the git. thank you!

Please sign in to comment.
Something went wrong with that request. Please try again.