Skip to content
This repository

fixed: prevent infinite loop in add-on dependency checks #2475

Merged
merged 1 commit into from over 1 year ago

5 participants

Memphiz Martijn Kaijser Arne Morten Kvarving ulion Voyager1
Deleted user
ghost commented

recently, we had a crash caused by two add-ons depending on each other.

Memphiz
Owner

looks solid

Martijn Kaijser
Owner

confirmed fixed on win32

xbmc/addons/AddonInstaller.cpp
... ... @@ -311,7 +313,8 @@ void CAddonInstaller::InstallFromXBMCRepo(const set<CStdString> &addonIDs)
311 313 Install(*i);
312 314 }
313 315
314   -bool CAddonInstaller::CheckDependencies(const AddonPtr &addon)
  316 +bool CAddonInstaller::CheckDependencies(const AddonPtr &addon,
  317 + std::vector<std::string>& preDeps)
2
ulion Collaborator
ulion added a note

how about add an overloaded method CheckDependencies(const AddonPtr &addon) to save other 3 places changes?

Arne Morten Kvarving
akva2 added a note

not sure it saves many lines but sure i can do that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Arne Morten Kvarving
akva2 commented

redone.

ulion
Collaborator
ulion commented

looks clean.

Martijn Kaijser MartijnKaijser merged commit b0825b1 into from
Voyager1 Voyager1 commented on the diff
xbmc/addons/AddonInstaller.cpp
((17 lines not shown))
345 350 return false;
  351 + }
  352 + preDeps.push_back(dep->ID());
1
Voyager1 Collaborator
Voyager1 added a note

I've come across a case where dep == NULL causing crash. Probably this call needs to be guarded by if (dep)...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Showing 1 unique commit by 1 author.

Mar 21, 2013
spiff fixed: prevent infinite loop in add-on dependency checks e137421
This page is out of date. Refresh to see the latest.
21 xbmc/addons/AddonInstaller.cpp
@@ -313,6 +313,14 @@ void CAddonInstaller::InstallFromXBMCRepo(const set<CStdString> &addonIDs)
313 313
314 314 bool CAddonInstaller::CheckDependencies(const AddonPtr &addon)
315 315 {
  316 + std::vector<std::string> preDeps;
  317 + preDeps.push_back(addon->ID());
  318 + return CheckDependencies(addon, preDeps);
  319 +}
  320 +
  321 +bool CAddonInstaller::CheckDependencies(const AddonPtr &addon,
  322 + std::vector<std::string>& preDeps)
  323 +{
316 324 if (!addon.get())
317 325 return true; // a NULL addon has no dependencies
318 326 ADDONDEPS deps = addon->GetDeps();
@@ -333,16 +341,15 @@ bool CAddonInstaller::CheckDependencies(const AddonPtr &addon)
333 341 return false;
334 342 }
335 343 }
336   - // prevent infinite loops
337   - if (dep && dep->ID() == addon->ID())
338   - {
339   - CLog::Log(LOGERROR, "Addon %s depends on itself, ignoring", addon->ID().c_str());
340   - return false;
341   - }
342 344 // at this point we have our dep, or the dep is optional (and we don't have it) so check that it's OK as well
343 345 // TODO: should we assume that installed deps are OK?
344   - if (dep && !CheckDependencies(dep))
  346 + if (dep &&
  347 + std::find(preDeps.begin(), preDeps.end(), dep->ID()) == preDeps.end() &&
  348 + !CheckDependencies(dep, preDeps))
  349 + {
345 350 return false;
  351 + }
  352 + preDeps.push_back(dep->ID());
346 353 }
347 354 return true;
348 355 }
10 xbmc/addons/AddonInstaller.h
@@ -122,6 +122,16 @@ class CAddonInstaller : public IJobCallback
122 122 */
123 123 bool DoInstall(const ADDON::AddonPtr &addon, const CStdString &hash = "", bool update = false, const CStdString &referer = "", bool background = true);
124 124
  125 + /*! \brief Check whether dependencies of an addon exist or are installable.
  126 + Iterates through the addon's dependencies, checking they're installed or installable.
  127 + Each dependency must also satisfies CheckDependencies in turn.
  128 + \param addon the addon to check
  129 + \param preDeps previous dependencies encountered during recursion. aids in avoiding infinite recursion
  130 + \return true if dependencies are available, false otherwise.
  131 + */
  132 + bool CheckDependencies(const ADDON::AddonPtr &addon,
  133 + std::vector<std::string>& preDeps);
  134 +
125 135 void PrunePackageCache();
126 136 int64_t EnumeratePackageFolder(std::map<CStdString,CFileItemList*>& result);
127 137

Tip: You can add notes to lines in a file. Hover to the left of a line to make a note

Something went wrong with that request. Please try again.