From 06626d6fbe66d1e11bb52b6639190ad7166904e1 Mon Sep 17 00:00:00 2001 From: Jonathan Marshall Date: Mon, 30 Jun 2014 11:14:28 +1200 Subject: [PATCH 01/13] [xmlutils] adds helper GetAttribute for retrieving a std::string version of an attribute --- xbmc/utils/XMLUtils.cpp | 11 +++++++++++ xbmc/utils/XMLUtils.h | 6 ++++++ 2 files changed, 17 insertions(+) diff --git a/xbmc/utils/XMLUtils.cpp b/xbmc/utils/XMLUtils.cpp index df747925d7376..07daa172715eb 100644 --- a/xbmc/utils/XMLUtils.cpp +++ b/xbmc/utils/XMLUtils.cpp @@ -268,6 +268,17 @@ bool XMLUtils::GetDateTime(const TiXmlNode* pRootNode, const char* strTag, CDate return false; } +std::string XMLUtils::GetAttribute(const TiXmlElement *element, const char *tag) +{ + if (element) + { + const char *attribute = element->Attribute(tag); + if (attribute) + return attribute; + } + return ""; +} + void XMLUtils::SetAdditiveString(TiXmlNode* pRootNode, const char *strTag, const CStdString& strSeparator, const CStdString& strValue) { std::vector list = StringUtils::Split(strValue, strSeparator); diff --git a/xbmc/utils/XMLUtils.h b/xbmc/utils/XMLUtils.h index 2884eea121d56..9285bee5d87a7 100644 --- a/xbmc/utils/XMLUtils.h +++ b/xbmc/utils/XMLUtils.h @@ -61,6 +61,12 @@ class XMLUtils static bool GetInt(const TiXmlNode* pRootNode, const char* strTag, int& iIntValue, const int min, const int max); static bool GetDate(const TiXmlNode* pRootNode, const char* strTag, CDateTime& date); static bool GetDateTime(const TiXmlNode* pRootNode, const char* strTag, CDateTime& dateTime); + /*! \brief Fetch a std::string copy of an attribute, if it exists. Cannot distinguish between empty and non-existent attributes. + \param element the element to query. + \param tag the name of the attribute. + \return the attribute, if it exists, else an empty string + */ + static std::string GetAttribute(const TiXmlElement *element, const char *tag); static void SetString(TiXmlNode* pRootNode, const char *strTag, const CStdString& strValue); static void SetAdditiveString(TiXmlNode* pRootNode, const char *strTag, const CStdString& strSeparator, const CStdString& strValue); From 4d160404d3bef0cd9d8e7ee05a8b2ab097c28cbb Mon Sep 17 00:00:00 2001 From: Jonathan Marshall Date: Tue, 1 Jul 2014 12:19:24 +1200 Subject: [PATCH 02/13] [xmlutils] use XMLUtils::GetAttribute() when assigning attributes directly to CStdString/std::string, as they may be NULL --- xbmc/LangInfo.cpp | 11 ++++---- xbmc/addons/GUIDialogAddonSettings.cpp | 10 ++++---- .../playercorefactory/PlayerCoreFactory.cpp | 7 +++--- .../playercorefactory/PlayerSelectionRule.cpp | 25 ++++++++++--------- xbmc/filesystem/LibraryDirectory.cpp | 5 ++-- xbmc/filesystem/RSSDirectory.cpp | 15 +++++------ xbmc/guilib/GUIControlFactory.cpp | 8 +++--- xbmc/guilib/GUIIncludes.cpp | 3 ++- xbmc/guilib/GUIStaticItem.cpp | 10 ++++---- xbmc/guilib/VisibleEffect.cpp | 7 +++--- xbmc/input/KeyboardLayoutConfiguration.cpp | 13 +++++----- xbmc/peripherals/Peripherals.cpp | 14 +++++------ xbmc/peripherals/devices/Peripheral.cpp | 5 ++-- xbmc/playlists/PlayListB4S.cpp | 5 ++-- xbmc/playlists/PlayListPLS.cpp | 9 ++++--- xbmc/playlists/PlayListWPL.cpp | 5 ++-- xbmc/settings/AdvancedSettings.cpp | 8 +++--- xbmc/settings/SkinSettings.cpp | 3 ++- xbmc/utils/Fanart.cpp | 7 +++--- xbmc/utils/ScraperParser.cpp | 3 ++- 20 files changed, 93 insertions(+), 80 deletions(-) diff --git a/xbmc/LangInfo.cpp b/xbmc/LangInfo.cpp index 5d2df4638a3dc..b1b7f9e6b4327 100644 --- a/xbmc/LangInfo.cpp +++ b/xbmc/LangInfo.cpp @@ -35,6 +35,7 @@ #include "utils/StringUtils.h" #include "utils/Weather.h" #include "utils/XBMCTinyXML.h" +#include "utils/XMLUtils.h" using namespace std; using namespace PVR; @@ -279,10 +280,10 @@ bool CLangInfo::Load(const std::string& strFileName, bool onlyCheckLanguage /*= const TiXmlNode *pCharSets = pRootElement->FirstChild("charsets"); if (pCharSets && !pCharSets->NoChildren()) { - const TiXmlNode *pGui = pCharSets->FirstChild("gui"); + const TiXmlElement *pGui = pCharSets->FirstChildElement("gui"); if (pGui && !pGui->NoChildren()) { - CStdString strForceUnicodeFont = ((TiXmlElement*) pGui)->Attribute("unicodefont"); + CStdString strForceUnicodeFont = XMLUtils::GetAttribute(pGui, "unicodefont"); if (strForceUnicodeFont.Equals("true")) m_defaultRegion.m_forceUnicodeFont=true; @@ -318,7 +319,7 @@ bool CLangInfo::Load(const std::string& strFileName, bool onlyCheckLanguage /*= while (pRegion) { CRegion region(m_defaultRegion); - region.m_strName=pRegion->Attribute("name"); + region.m_strName = XMLUtils::GetAttribute(pRegion, "name"); if (region.m_strName.empty()) region.m_strName="N/A"; @@ -346,8 +347,8 @@ bool CLangInfo::Load(const std::string& strFileName, bool onlyCheckLanguage /*= if (pTime && !pTime->NoChildren()) { region.m_strTimeFormat=pTime->FirstChild()->Value(); - region.m_strMeridiemSymbols[MERIDIEM_SYMBOL_AM]=pTime->Attribute("symbolAM"); - region.m_strMeridiemSymbols[MERIDIEM_SYMBOL_PM]=pTime->Attribute("symbolPM"); + region.m_strMeridiemSymbols[MERIDIEM_SYMBOL_AM] = XMLUtils::GetAttribute(pTime, "symbolAM"); + region.m_strMeridiemSymbols[MERIDIEM_SYMBOL_PM] = XMLUtils::GetAttribute(pTime, "symbolPM"); } const TiXmlNode *pTempUnit=pRegion->FirstChild("tempunit"); diff --git a/xbmc/addons/GUIDialogAddonSettings.cpp b/xbmc/addons/GUIDialogAddonSettings.cpp index aeffbe3975897..66ce8549af417 100644 --- a/xbmc/addons/GUIDialogAddonSettings.cpp +++ b/xbmc/addons/GUIDialogAddonSettings.cpp @@ -306,7 +306,7 @@ bool CGUIDialogAddonSettings::ShowVirtualKeyboard(int iControl) } else if (source) { - valuesVec = GetFileEnumValues(source, setting->Attribute("mask"), setting->Attribute("option")); + valuesVec = GetFileEnumValues(source, XMLUtils::GetAttribute(setting, "mask"), XMLUtils::GetAttribute(setting, "option")); } for (unsigned int i = 0; i < valuesVec.size(); i++) @@ -503,7 +503,7 @@ void CGUIDialogAddonSettings::UpdateFromControls() const TiXmlElement *setting = GetFirstSetting(); while (setting) { - CStdString id = setting->Attribute("id"); + const std::string id = XMLUtils::GetAttribute(setting, "id"); const char *type = setting->Attribute("type"); const CGUIControl* control = GetControl(controlID++); @@ -526,7 +526,7 @@ void CGUIDialogAddonSettings::UpdateFromControls() break; case CGUIControl::GUICONTROL_SETTINGS_SLIDER: { - CStdString option = setting->Attribute("option"); + CStdString option = XMLUtils::GetAttribute(setting, "option"); if (option.size() == 0 || StringUtils::EqualsNoCase(option, "float")) value = StringUtils::Format("%f", ((CGUISettingsSliderControl *)control)->GetFloatValue()); else @@ -801,7 +801,7 @@ void CGUIDialogAddonSettings::CreateControls() ((CGUISpinControlEx *)pControl)->SetText(label); ((CGUISpinControlEx *)pControl)->SetFloatValue(1.0f); - vector items = GetFileEnumValues(values, setting->Attribute("mask"), setting->Attribute("option")); + vector items = GetFileEnumValues(values, XMLUtils::GetAttribute(setting, "mask"), XMLUtils::GetAttribute(setting, "option")); for (unsigned int i = 0; i < items.size(); ++i) { ((CGUISpinControlEx *)pControl)->AddLabel(items[i], i); @@ -854,7 +854,7 @@ void CGUIDialogAddonSettings::CreateControls() float fMin = 0.0f; float fMax = 100.0f; float fInc = 1.0f; - vector range = StringUtils::Split(setting->Attribute("range"), ","); + vector range = StringUtils::Split(XMLUtils::GetAttribute(setting, "range"), ","); if (range.size() > 1) { fMin = (float)atof(range[0].c_str()); diff --git a/xbmc/cores/playercorefactory/PlayerCoreFactory.cpp b/xbmc/cores/playercorefactory/PlayerCoreFactory.cpp index 570c5eb1dcb00..72630e4505408 100644 --- a/xbmc/cores/playercorefactory/PlayerCoreFactory.cpp +++ b/xbmc/cores/playercorefactory/PlayerCoreFactory.cpp @@ -39,6 +39,7 @@ #include "guilib/LocalizeStrings.h" #include "cores/AudioEngine/AEFactory.h" #include "utils/StringUtils.h" +#include "utils/XMLUtils.h" #define PLAYERCOREFACTORY_XML "playercorefactory.xml" @@ -360,9 +361,9 @@ bool CPlayerCoreFactory::LoadConfiguration(const std::string &file, bool clear) TiXmlElement* pPlayer = pPlayers->FirstChildElement("player"); while (pPlayer) { - CStdString name = pPlayer->Attribute("name"); - CStdString type = pPlayer->Attribute("type"); - if (type.length() == 0) type = name; + CStdString name = XMLUtils::GetAttribute(pPlayer, "name"); + CStdString type = XMLUtils::GetAttribute(pPlayer, "type"); + if (type.empty()) type = name; StringUtils::ToLower(type); EPLAYERCORES eCore = EPC_NONE; diff --git a/xbmc/cores/playercorefactory/PlayerSelectionRule.cpp b/xbmc/cores/playercorefactory/PlayerSelectionRule.cpp index 63a0125a3e497..de48d8d7384da 100644 --- a/xbmc/cores/playercorefactory/PlayerSelectionRule.cpp +++ b/xbmc/cores/playercorefactory/PlayerSelectionRule.cpp @@ -26,6 +26,7 @@ #include "utils/log.h" #include "utils/RegExp.h" #include "utils/XBMCTinyXML.h" +#include "utils/XMLUtils.h" CPlayerSelectionRule::CPlayerSelectionRule(TiXmlElement* pRule) { @@ -37,8 +38,8 @@ CPlayerSelectionRule::~CPlayerSelectionRule() void CPlayerSelectionRule::Initialize(TiXmlElement* pRule) { - m_name = pRule->Attribute("name"); - if (!m_name || m_name.empty()) + m_name = XMLUtils::GetAttribute(pRule, "name"); + if (m_name.empty()) m_name = "un-named"; CLog::Log(LOGDEBUG, "CPlayerSelectionRule::Initialize: creating rule: %s", m_name.c_str()); @@ -53,16 +54,16 @@ void CPlayerSelectionRule::Initialize(TiXmlElement* pRule) m_tDVDFile = GetTristate(pRule->Attribute("dvdfile")); m_tDVDImage = GetTristate(pRule->Attribute("dvdimage")); - m_protocols = pRule->Attribute("protocols"); - m_fileTypes = pRule->Attribute("filetypes"); - m_mimeTypes = pRule->Attribute("mimetypes"); - m_fileName = pRule->Attribute("filename"); + m_protocols = XMLUtils::GetAttribute(pRule, "protocols"); + m_fileTypes = XMLUtils::GetAttribute(pRule, "filetypes"); + m_mimeTypes = XMLUtils::GetAttribute(pRule, "mimetypes"); + m_fileName = XMLUtils::GetAttribute(pRule, "filename"); - m_audioCodec = pRule->Attribute("audiocodec"); - m_audioChannels = pRule->Attribute("audiochannels"); - m_videoCodec = pRule->Attribute("videocodec"); - m_videoResolution = pRule->Attribute("videoresolution"); - m_videoAspect = pRule->Attribute("videoaspect"); + m_audioCodec = XMLUtils::GetAttribute(pRule, "audiocodec"); + m_audioChannels = XMLUtils::GetAttribute(pRule, "audiochannels"); + m_videoCodec = XMLUtils::GetAttribute(pRule, "videocodec"); + m_videoResolution = XMLUtils::GetAttribute(pRule, "videoresolution"); + m_videoAspect = XMLUtils::GetAttribute(pRule, "videoaspect"); m_bStreamDetails = m_audioCodec.length() > 0 || m_audioChannels.length() > 0 || m_videoCodec.length() > 0 || m_videoResolution.length() > 0 || m_videoAspect.length() > 0; @@ -72,7 +73,7 @@ void CPlayerSelectionRule::Initialize(TiXmlElement* pRule) CLog::Log(LOGWARNING, "CPlayerSelectionRule::Initialize: rule: %s needs media flagging, which is disabled", m_name.c_str()); } - m_playerName = pRule->Attribute("player"); + m_playerName = XMLUtils::GetAttribute(pRule, "player"); m_playerCoreId = 0; TiXmlElement* pSubRule = pRule->FirstChildElement("rule"); diff --git a/xbmc/filesystem/LibraryDirectory.cpp b/xbmc/filesystem/LibraryDirectory.cpp index cd0a2af58e290..b33ececc133b7 100644 --- a/xbmc/filesystem/LibraryDirectory.cpp +++ b/xbmc/filesystem/LibraryDirectory.cpp @@ -56,7 +56,7 @@ bool CLibraryDirectory::GetDirectory(const CURL& url, CFileItemList &items) TiXmlElement *node = LoadXML(libNode); if (node) { - CStdString type = node->Attribute("type"); + CStdString type = XMLUtils::GetAttribute(node, "type"); if (type == "filter") { CSmartPlaylist playlist; @@ -156,8 +156,7 @@ TiXmlElement *CLibraryDirectory::LoadXML(const std::string &xmlFile) return NULL; // check the condition - std::string condition; - xml->QueryStringAttribute("visible", &condition); + std::string condition = XMLUtils::GetAttribute(xml, "visible"); if (condition.empty() || g_infoManager.EvaluateBool(condition)) return xml; diff --git a/xbmc/filesystem/RSSDirectory.cpp b/xbmc/filesystem/RSSDirectory.cpp index bf1b34e9eb0eb..c2763c23fa4d1 100644 --- a/xbmc/filesystem/RSSDirectory.cpp +++ b/xbmc/filesystem/RSSDirectory.cpp @@ -26,6 +26,7 @@ #include "settings/Settings.h" #include "utils/URIUtils.h" #include "utils/XBMCTinyXML.h" +#include "utils/XMLUtils.h" #include "utils/HTMLUtil.h" #include "utils/StringUtils.h" #include "video/VideoInfoTag.h" @@ -121,8 +122,8 @@ static void ParseItemMRSS(CFileItem* item, SResources& resources, TiXmlElement* { SResource res; res.tag = "media:content"; - res.mime = item_child->Attribute("type"); - res.path = item_child->Attribute("url"); + res.mime = XMLUtils::GetAttribute(item_child, "type"); + res.path = XMLUtils::GetAttribute(item_child, "url"); if(item_child->Attribute("width")) res.width = atoi(item_child->Attribute("width")); if(item_child->Attribute("height")) @@ -166,7 +167,7 @@ static void ParseItemMRSS(CFileItem* item, SResources& resources, TiXmlElement* return; CStdString description = text; - if(CStdString(item_child->Attribute("type")) == "html") + if(XMLUtils::GetAttribute(item_child, "type") == "html") HTML::CHTMLUtil::RemoveTags(description); item->SetProperty("description", description); } @@ -175,7 +176,7 @@ static void ParseItemMRSS(CFileItem* item, SResources& resources, TiXmlElement* if(text.empty()) return; - CStdString scheme = item_child->Attribute("scheme"); + CStdString scheme = XMLUtils::GetAttribute(item_child, "scheme"); /* okey this is silly, boxee what did you think?? */ if (scheme == "urn:boxee:genre") @@ -202,7 +203,7 @@ static void ParseItemMRSS(CFileItem* item, SResources& resources, TiXmlElement* } else if(name == "rating") { - CStdString scheme = item_child->Attribute("scheme"); + CStdString scheme = XMLUtils::GetAttribute(item_child, "scheme"); if(scheme == "urn:user") vtag->m_fRating = (float)atof(text.c_str()); else @@ -210,7 +211,7 @@ static void ParseItemMRSS(CFileItem* item, SResources& resources, TiXmlElement* } else if(name == "credit") { - CStdString role = item_child->Attribute("role"); + CStdString role = XMLUtils::GetAttribute(item_child, "role"); if (role == "director") vtag->m_director.push_back(text); else if(role == "author" @@ -321,7 +322,7 @@ static void ParseItemVoddler(CFileItem* item, SResources& resources, TiXmlElemen SResource res; res.tag = "voddler:trailer"; - res.mime = element->Attribute("type"); + res.mime = XMLUtils::GetAttribute(element, "type"); res.path = text; resources.push_back(res); } diff --git a/xbmc/guilib/GUIControlFactory.cpp b/xbmc/guilib/GUIControlFactory.cpp index b80372e8fcf2f..e8c0db024522e 100644 --- a/xbmc/guilib/GUIControlFactory.cpp +++ b/xbmc/guilib/GUIControlFactory.cpp @@ -358,8 +358,8 @@ bool CGUIControlFactory::GetTexture(const TiXmlNode* pRootNode, const char* strT if (flipX && strcmpi(flipX, "true") == 0) image.orientation = 1; const char *flipY = pNode->Attribute("flipy"); if (flipY && strcmpi(flipY, "true") == 0) image.orientation = 3 - image.orientation; // either 3 or 2 - image.diffuse = pNode->Attribute("diffuse"); - image.diffuseColor.Parse(pNode->Attribute("colordiffuse"), 0); + image.diffuse = XMLUtils::GetAttribute(pNode, "diffuse"); + image.diffuseColor.Parse(XMLUtils::GetAttribute(pNode, "colordiffuse"), 0); const char *background = pNode->Attribute("background"); if (background && strnicmp(background, "true", 4) == 0) image.useLarge = true; @@ -503,7 +503,7 @@ bool CGUIControlFactory::GetActions(const TiXmlNode* pRootNode, const char* strT if (pElement->FirstChild()) { CGUIAction::cond_action_pair pair; - pair.condition = pElement->Attribute("condition"); + pair.condition = XMLUtils::GetAttribute(pElement, "condition"); pair.action = pElement->FirstChild()->Value(); action.m_actions.push_back(pair); } @@ -582,7 +582,7 @@ bool CGUIControlFactory::GetInfoLabelFromElement(const TiXmlElement *element, CG if (label.empty() || label == "-") return false; - CStdString fallback = element->Attribute("fallback"); + CStdString fallback = XMLUtils::GetAttribute(element, "fallback"); if (StringUtils::IsNaturalNumber(label)) label = g_localizeStrings.Get(atoi(label)); if (StringUtils::IsNaturalNumber(fallback)) diff --git a/xbmc/guilib/GUIIncludes.cpp b/xbmc/guilib/GUIIncludes.cpp index 5062a1ead8cbc..0ebf072a3cf8a 100644 --- a/xbmc/guilib/GUIIncludes.cpp +++ b/xbmc/guilib/GUIIncludes.cpp @@ -23,6 +23,7 @@ #include "GUIInfoManager.h" #include "utils/log.h" #include "utils/XBMCTinyXML.h" +#include "utils/XMLUtils.h" #include "utils/StringUtils.h" #include "interfaces/info/SkinVariable.h" @@ -207,7 +208,7 @@ void CGUIIncludes::ResolveIncludesForNode(TiXmlElement *node, std::mapValueStr() == "control") { - type = node->Attribute("type"); + type = XMLUtils::GetAttribute(node, "type"); map::const_iterator it = m_defaults.find(type); if (it != m_defaults.end()) { diff --git a/xbmc/guilib/GUIStaticItem.cpp b/xbmc/guilib/GUIStaticItem.cpp index d371691cee3d1..c4d6f530b52bb 100644 --- a/xbmc/guilib/GUIStaticItem.cpp +++ b/xbmc/guilib/GUIStaticItem.cpp @@ -59,7 +59,7 @@ CGUIStaticItem::CGUIStaticItem(const TiXmlElement *item, int parentID) : CFileIt const TiXmlElement *property = item->FirstChildElement("property"); while (property) { - CStdString name = property->Attribute("name"); + CStdString name = XMLUtils::GetAttribute(property, "name"); CGUIInfoLabel prop; if (!name.empty() && CGUIControlFactory::GetInfoLabelFromElement(property, prop, parentID)) { @@ -73,10 +73,10 @@ CGUIStaticItem::CGUIStaticItem(const TiXmlElement *item, int parentID) : CFileIt else { CStdString label, label2, thumb, icon; - label = item->Attribute("label"); label = CGUIControlFactory::FilterLabel(label); - label2 = item->Attribute("label2"); label2 = CGUIControlFactory::FilterLabel(label2); - thumb = item->Attribute("thumb"); thumb = CGUIControlFactory::FilterLabel(thumb); - icon = item->Attribute("icon"); icon = CGUIControlFactory::FilterLabel(icon); + label = XMLUtils::GetAttribute(item, "label"); label = CGUIControlFactory::FilterLabel(label); + label2 = XMLUtils::GetAttribute(item, "label2"); label2 = CGUIControlFactory::FilterLabel(label2); + thumb = XMLUtils::GetAttribute(item, "thumb"); thumb = CGUIControlFactory::FilterLabel(thumb); + icon = XMLUtils::GetAttribute(item, "icon"); icon = CGUIControlFactory::FilterLabel(icon); const char *id = item->Attribute("id"); SetLabel(CGUIInfoLabel::GetLabel(label, parentID)); SetPath(item->FirstChild()->Value()); diff --git a/xbmc/guilib/VisibleEffect.cpp b/xbmc/guilib/VisibleEffect.cpp index 22df1670ac4b5..ea855a1d5d6aa 100644 --- a/xbmc/guilib/VisibleEffect.cpp +++ b/xbmc/guilib/VisibleEffect.cpp @@ -25,6 +25,7 @@ #include "utils/StringUtils.h" #include "Tween.h" #include "utils/XBMCTinyXML.h" +#include "utils/XMLUtils.h" using namespace std; @@ -627,7 +628,7 @@ void CAnimation::Create(const TiXmlElement *node, const CRect &rect, int context CStdString type = node->FirstChild()->Value(); m_type = ANIM_TYPE_CONDITIONAL; if (effect) // new layout - type = node->Attribute("type"); + type = XMLUtils::GetAttribute(node, "type"); if (StringUtils::StartsWithNoCase(type, "visible")) m_type = ANIM_TYPE_VISIBLE; else if (type.Equals("hidden")) m_type = ANIM_TYPE_HIDDEN; @@ -656,7 +657,7 @@ void CAnimation::Create(const TiXmlElement *node, const CRect &rect, int context if (!effect) { // old layout: // focus - CStdString type = node->Attribute("effect"); + CStdString type = XMLUtils::GetAttribute(node, "effect"); AddEffect(type, node, rect); } while (effect) @@ -665,7 +666,7 @@ void CAnimation::Create(const TiXmlElement *node, const CRect &rect, int context // // ... // - CStdString type = effect->Attribute("type"); + CStdString type = XMLUtils::GetAttribute(effect, "type"); AddEffect(type, effect, rect); effect = effect->NextSiblingElement("effect"); } diff --git a/xbmc/input/KeyboardLayoutConfiguration.cpp b/xbmc/input/KeyboardLayoutConfiguration.cpp index 49a2403c0aff6..779c14f83c7ab 100644 --- a/xbmc/input/KeyboardLayoutConfiguration.cpp +++ b/xbmc/input/KeyboardLayoutConfiguration.cpp @@ -21,6 +21,7 @@ #include "KeyboardLayoutConfiguration.h" #include "utils/CharsetConverter.h" #include "utils/XBMCTinyXML.h" +#include "utils/XMLUtils.h" using namespace std; CKeyboardLayoutConfiguration g_keyboardLayoutConfiguration; @@ -94,9 +95,9 @@ void CKeyboardLayoutConfiguration::readCharMapFromXML(const TiXmlElement* pXMLMa const TiXmlElement* pEntry = pXMLMap->FirstChildElement(); while (pEntry) { - CStdString strInChar = pEntry->Attribute("inchar"); - CStdString strOutChar = pEntry->Attribute("outchar"); - if (strInChar.length() > 0 && strOutChar.length() > 0) + CStdString strInChar = XMLUtils::GetAttribute(pEntry, "inchar"); + CStdString strOutChar = XMLUtils::GetAttribute(pEntry, "outchar"); + if (!strInChar.empty() && !strOutChar.empty()) { CStdStringW fromStr; g_charsetConverter.utf8ToW(strInChar, fromStr); @@ -132,9 +133,9 @@ void CKeyboardLayoutConfiguration::readByteMapFromXML(const TiXmlElement* pXMLMa const TiXmlElement* pEntry = pXMLMap->FirstChildElement(); while (pEntry) { - CStdString strInHex = pEntry->Attribute("inhex"); - CStdString strOutChar = pEntry->Attribute("outchar"); - if (strInHex.length() > 0 && strOutChar.length() > 0) + CStdString strInHex = XMLUtils::GetAttribute(pEntry, "inhex"); + CStdString strOutChar = XMLUtils::GetAttribute(pEntry, "outchar"); + if (!strInHex.empty() && !strOutChar.empty()) { CStdString hexValue = strInHex; CStdStringW toStr; diff --git a/xbmc/peripherals/Peripherals.cpp b/xbmc/peripherals/Peripherals.cpp index 2abac7d553447..d75542924c13e 100644 --- a/xbmc/peripherals/Peripherals.cpp +++ b/xbmc/peripherals/Peripherals.cpp @@ -452,9 +452,9 @@ bool CPeripherals::LoadMappings(void) } } - mapping.m_busType = PeripheralTypeTranslator::GetBusTypeFromString(currentNode->Attribute("bus")); - mapping.m_class = PeripheralTypeTranslator::GetTypeFromString(currentNode->Attribute("class")); - mapping.m_mappedTo = PeripheralTypeTranslator::GetTypeFromString(currentNode->Attribute("mapTo")); + mapping.m_busType = PeripheralTypeTranslator::GetBusTypeFromString(XMLUtils::GetAttribute(currentNode, "bus")); + mapping.m_class = PeripheralTypeTranslator::GetTypeFromString(XMLUtils::GetAttribute(currentNode, "class")); + mapping.m_mappedTo = PeripheralTypeTranslator::GetTypeFromString(XMLUtils::GetAttribute(currentNode, "mapTo")); GetSettingsFromMappingsFile(currentNode, mapping.m_settings); m_mappings.push_back(mapping); @@ -472,11 +472,11 @@ void CPeripherals::GetSettingsFromMappingsFile(TiXmlElement *xmlNode, mapAttribute("key")); + CStdString strKey = XMLUtils::GetAttribute(currentNode, "key"); if (strKey.empty()) continue; - CStdString strSettingsType(currentNode->Attribute("type")); + CStdString strSettingsType = XMLUtils::GetAttribute(currentNode, "type"); int iLabelId = currentNode->Attribute("label") ? atoi(currentNode->Attribute("label")) : -1; bool bConfigurable = (!currentNode->Attribute("configurable") || strcmp(currentNode->Attribute("configurable"), "") == 0 || @@ -508,7 +508,7 @@ void CPeripherals::GetSettingsFromMappingsFile(TiXmlElement *xmlNode, mapAttribute("lvalues")); + CStdString strEnums = XMLUtils::GetAttribute(currentNode, "lvalues"); if (!strEnums.empty()) { vector< pair > enums; @@ -522,7 +522,7 @@ void CPeripherals::GetSettingsFromMappingsFile(TiXmlElement *xmlNode, mapAttribute("value")); + CStdString strValue = XMLUtils::GetAttribute(currentNode, "value"); setting = new CSettingString(strKey, iLabelId, strValue); } diff --git a/xbmc/peripherals/devices/Peripheral.cpp b/xbmc/peripherals/devices/Peripheral.cpp index b6e28c1a116fd..b1eaa1fa27b21 100644 --- a/xbmc/peripherals/devices/Peripheral.cpp +++ b/xbmc/peripherals/devices/Peripheral.cpp @@ -24,6 +24,7 @@ #include "utils/StringUtils.h" #include "settings/lib/Setting.h" #include "utils/XBMCTinyXML.h" +#include "utils/XMLUtils.h" #include "utils/URIUtils.h" #include "guilib/LocalizeStrings.h" @@ -491,8 +492,8 @@ void CPeripheral::LoadPersistedSettings(void) const TiXmlElement *setting = doc.RootElement()->FirstChildElement("setting"); while (setting) { - CStdString strId(setting->Attribute("id")); - CStdString strValue(setting->Attribute("value")); + CStdString strId = XMLUtils::GetAttribute(setting, "id"); + CStdString strValue = XMLUtils::GetAttribute(setting, "value"); SetSetting(strId, strValue); setting = setting->NextSiblingElement("setting"); diff --git a/xbmc/playlists/PlayListB4S.cpp b/xbmc/playlists/PlayListB4S.cpp index 3e241bd2944fc..e1c6db79ad20c 100644 --- a/xbmc/playlists/PlayListB4S.cpp +++ b/xbmc/playlists/PlayListB4S.cpp @@ -26,6 +26,7 @@ #include "filesystem/File.h" #include "utils/log.h" #include "utils/URIUtils.h" +#include "utils/XMLUtils.h" using namespace XFILE; using namespace PLAYLIST; @@ -71,14 +72,14 @@ bool CPlayListB4S::LoadData(istream& stream) TiXmlElement* pPlayListElement = pRootElement->FirstChildElement("playlist"); if (!pPlayListElement ) return false; - m_strPlayListName = pPlayListElement->Attribute("label"); + m_strPlayListName = XMLUtils::GetAttribute(pPlayListElement, "label"); TiXmlElement* pEntryElement = pPlayListElement->FirstChildElement("entry"); if (!pEntryElement) return false; while (pEntryElement) { - CStdString strFileName = pEntryElement->Attribute("Playstring"); + CStdString strFileName = XMLUtils::GetAttribute(pEntryElement, "Playstring"); size_t iColon = strFileName.find(":"); if (iColon != std::string::npos) { diff --git a/xbmc/playlists/PlayListPLS.cpp b/xbmc/playlists/PlayListPLS.cpp index 327707cc7f2ac..3c1d624d3ab1a 100644 --- a/xbmc/playlists/PlayListPLS.cpp +++ b/xbmc/playlists/PlayListPLS.cpp @@ -29,6 +29,7 @@ #include "utils/log.h" #include "utils/URIUtils.h" #include "utils/XBMCTinyXML.h" +#include "utils/XMLUtils.h" using namespace std; using namespace XFILE; @@ -358,8 +359,8 @@ bool CPlayListASX::LoadData(istream& stream) while (pRef) { // multiple references may apear for one entry // duration may exist on this level too - value = pRef->Attribute("href"); - if (value != "") + value = XMLUtils::GetAttribute(pRef, "href"); + if (!value.empty()) { if(title.empty()) title = value; @@ -374,8 +375,8 @@ bool CPlayListASX::LoadData(istream& stream) } else if (value == "entryref") { - value = pElement->Attribute("href"); - if (value != "") + value = XMLUtils::GetAttribute(pElement, "href"); + if (!value.empty()) { // found an entryref, let's try loading that url auto_ptr playlist(CPlayListFactory::Create(value)); if (NULL != playlist.get()) diff --git a/xbmc/playlists/PlayListWPL.cpp b/xbmc/playlists/PlayListWPL.cpp index e121d1c560b91..851ac8f526c2c 100644 --- a/xbmc/playlists/PlayListWPL.cpp +++ b/xbmc/playlists/PlayListWPL.cpp @@ -25,6 +25,7 @@ #include "filesystem/File.h" #include "utils/log.h" #include "utils/URIUtils.h" +#include "utils/XMLUtils.h" using namespace XFILE; using namespace PLAYLIST; @@ -89,8 +90,8 @@ bool CPlayListWPL::LoadData(istream& stream) if (!pMediaElement) return false; while (pMediaElement) { - CStdString strFileName = pMediaElement->Attribute("src"); - if (strFileName.size()) + CStdString strFileName = XMLUtils::GetAttribute(pMediaElement, "src"); + if (!strFileName.empty()) { strFileName = URIUtils::SubstitutePath(strFileName); CUtil::GetQualifiedFilename(m_strBasePath, strFileName); diff --git a/xbmc/settings/AdvancedSettings.cpp b/xbmc/settings/AdvancedSettings.cpp index f8d7e1e44dca9..6e4e2b8f1f937 100644 --- a/xbmc/settings/AdvancedSettings.cpp +++ b/xbmc/settings/AdvancedSettings.cpp @@ -1071,7 +1071,7 @@ void CAdvancedSettings::ParseSettingsFile(const CStdString &file) TiXmlElement* element = pHostEntries->FirstChildElement("entry"); while(element) { - CStdString name = element->Attribute("name"); + CStdString name = XMLUtils::GetAttribute(element, "name"); CStdString value; if(element->GetText()) value = element->GetText(); @@ -1238,12 +1238,12 @@ void CAdvancedSettings::GetCustomTVRegexps(TiXmlElement *pRootElement, SETTINGS_ int iDefaultSeason = 1; if (pRegExp->ToElement()) { - CStdString byDate = pRegExp->ToElement()->Attribute("bydate"); - if(byDate && stricmp(byDate, "true") == 0) + CStdString byDate = XMLUtils::GetAttribute(pRegExp->ToElement(), "bydate"); + if (stricmp(byDate, "true") == 0) { bByDate = true; } - CStdString defaultSeason = pRegExp->ToElement()->Attribute("defaultseason"); + CStdString defaultSeason = XMLUtils::GetAttribute(pRegExp->ToElement(), "defaultseason"); if(!defaultSeason.empty()) { iDefaultSeason = atoi(defaultSeason.c_str()); diff --git a/xbmc/settings/SkinSettings.cpp b/xbmc/settings/SkinSettings.cpp index 13aba27e2f671..66a273ccce3b0 100644 --- a/xbmc/settings/SkinSettings.cpp +++ b/xbmc/settings/SkinSettings.cpp @@ -27,6 +27,7 @@ #include "utils/log.h" #include "utils/StringUtils.h" #include "utils/XBMCTinyXML.h" +#include "utils/XMLUtils.h" #define XML_SKINSETTINGS "skinsettings" #define XML_SETTING "setting" @@ -208,7 +209,7 @@ bool CSkinSettings::Load(const TiXmlNode *settings) const TiXmlElement *pChild = pElement->FirstChildElement(XML_SETTING); while (pChild) { - CStdString settingName = pChild->Attribute(XML_ATTR_NAME); + std::string settingName = XMLUtils::GetAttribute(pChild, XML_ATTR_NAME); if (pChild->Attribute("type") && StringUtils::EqualsNoCase(pChild->Attribute(XML_ATTR_TYPE), "string")) { // string setting CSkinString string; diff --git a/xbmc/utils/Fanart.cpp b/xbmc/utils/Fanart.cpp index 19313758477d1..e713b59e0406b 100644 --- a/xbmc/utils/Fanart.cpp +++ b/xbmc/utils/Fanart.cpp @@ -20,6 +20,7 @@ #include "Fanart.h" #include "utils/XBMCTinyXML.h" +#include "utils/XMLUtils.h" #include "URIUtils.h" #include "StringUtils.h" @@ -62,7 +63,7 @@ bool CFanart::Unpack() TiXmlElement *fanart = doc.FirstChildElement("fanart"); while (fanart) { - CStdString url = fanart->Attribute("url"); + CStdString url = XMLUtils::GetAttribute(fanart, "url"); TiXmlElement *fanartThumb = fanart->FirstChildElement("thumb"); while (fanartThumb) { @@ -79,8 +80,8 @@ bool CFanart::Unpack() if (fanartThumb->Attribute("preview")) data.strPreview = URIUtils::AddFileToFolder(url, fanartThumb->Attribute("preview")); } - data.strResolution = fanartThumb->Attribute("dim"); - ParseColors(fanartThumb->Attribute("colors"), data.strColors); + data.strResolution = XMLUtils::GetAttribute(fanartThumb, "dim"); + ParseColors(XMLUtils::GetAttribute(fanartThumb, "colors"), data.strColors); m_fanart.push_back(data); fanartThumb = fanartThumb->NextSiblingElement("thumb"); } diff --git a/xbmc/utils/ScraperParser.cpp b/xbmc/utils/ScraperParser.cpp index 023f7527d6883..5142e4ad233a2 100644 --- a/xbmc/utils/ScraperParser.cpp +++ b/xbmc/utils/ScraperParser.cpp @@ -31,6 +31,7 @@ #include "CharsetConverter.h" #include "utils/StringUtils.h" #include "utils/XSLTUtils.h" +#include "utils/XMLUtils.h" #include #include @@ -193,7 +194,7 @@ void CScraperParser::ReplaceBuffers(CStdString& strDest) void CScraperParser::ParseExpression(const CStdString& input, CStdString& dest, TiXmlElement* element, bool bAppend) { - CStdString strOutput = element->Attribute("output"); + CStdString strOutput = XMLUtils::GetAttribute(element, "output"); TiXmlElement* pExpression = element->FirstChildElement("expression"); if (pExpression) From 2ce4cccd07f6d9d3a143a638ae6659ffc6f5341d Mon Sep 17 00:00:00 2001 From: Jonathan Marshall Date: Tue, 1 Jul 2014 12:55:27 +1200 Subject: [PATCH 03/13] [xml] fix potential null pointer dereference when reading attributes --- xbmc/addons/AddonCallbacksAddon.cpp | 2 +- xbmc/addons/AddonDll.h | 2 +- xbmc/addons/GUIDialogAddonSettings.cpp | 18 ++++++++++-------- xbmc/input/ButtonTranslator.cpp | 4 ++-- 4 files changed, 14 insertions(+), 12 deletions(-) diff --git a/xbmc/addons/AddonCallbacksAddon.cpp b/xbmc/addons/AddonCallbacksAddon.cpp index 776b5adf37fe7..41fc8a676daff 100644 --- a/xbmc/addons/AddonCallbacksAddon.cpp +++ b/xbmc/addons/AddonCallbacksAddon.cpp @@ -199,7 +199,7 @@ bool CAddonCallbacksAddon::GetAddonSetting(void *addonData, const char *strSetti const char *id = setting->Attribute("id"); const char *type = setting->Attribute("type"); - if (strcmpi(id, strSettingName) == 0 && type) + if (id && strcmpi(id, strSettingName) == 0 && type) { if (strcmpi(type, "text") == 0 || strcmpi(type, "ipaddress") == 0 || strcmpi(type, "folder") == 0 || strcmpi(type, "action") == 0 || diff --git a/xbmc/addons/AddonDll.h b/xbmc/addons/AddonDll.h index 5c0c3b552da82..2418377788e8c 100644 --- a/xbmc/addons/AddonDll.h +++ b/xbmc/addons/AddonDll.h @@ -479,7 +479,7 @@ ADDON_STATUS CAddonDll::TransferSettings() const char *type = setting->Attribute("type"); const char *option = setting->Attribute("option"); - if (type) + if (id && type) { if (strcmpi(type,"sep") == 0 || strcmpi(type,"lsep") == 0) { diff --git a/xbmc/addons/GUIDialogAddonSettings.cpp b/xbmc/addons/GUIDialogAddonSettings.cpp index 66ce8549af417..e4b41b3c23f96 100644 --- a/xbmc/addons/GUIDialogAddonSettings.cpp +++ b/xbmc/addons/GUIDialogAddonSettings.cpp @@ -164,7 +164,8 @@ bool CGUIDialogAddonSettings::OnAction(const CAction& action) { const char* id = setting->Attribute("id"); const char* value = setting->Attribute("default"); - m_settings[id] = value; + if (id && value) + m_settings[id] = value; CreateControls(); CGUIMessage msg(GUI_MSG_SETFOCUS,GetID(),iControl); OnMessage(msg); @@ -234,10 +235,11 @@ bool CGUIDialogAddonSettings::ShowVirtualKeyboard(int iControl) if (controlId == iControl) { const CGUIControl* control = GetControl(controlId); - if (control->GetControlType() == CGUIControl::GUICONTROL_BUTTON) + const char *id = setting->Attribute("id"); + const char *type = setting->Attribute("type"); + if (control && control->GetControlType() == CGUIControl::GUICONTROL_BUTTON && + id && type) { - const char *id = setting->Attribute("id"); - const char *type = setting->Attribute("type"); const char *option = setting->Attribute("option"); const char *source = setting->Attribute("source"); CStdString value = m_buttonValues[id]; @@ -412,7 +414,7 @@ bool CGUIDialogAddonSettings::ShowVirtualKeyboard(int iControl) } else if (strcmpi(type, "action") == 0) { - CStdString action = setting->Attribute("action"); + CStdString action = XMLUtils::GetAttribute(setting, "action"); if (!action.empty()) { // replace $CWD with the url of plugin/script @@ -686,7 +688,7 @@ void CGUIDialogAddonSettings::CreateControls() if (sort && (strcmp(sort, "yes") == 0)) bSort=true; - if (type) + if (id && type) { bool isAddonSetting = false; if (strcmpi(type, "text") == 0 || strcmpi(type, "ipaddress") == 0 || @@ -867,10 +869,10 @@ void CGUIDialogAddonSettings::CreateControls() fMax = (float)atof(range[1].c_str()); } - CStdString option = setting->Attribute("option"); + std::string option = XMLUtils::GetAttribute(setting, "option"); int iType=0; - if (option.size() == 0 || StringUtils::EqualsNoCase(option, "float")) + if (option.empty() || StringUtils::EqualsNoCase(option, "float")) iType = SLIDER_CONTROL_TYPE_FLOAT; else if (StringUtils::EqualsNoCase(option, "int")) iType = SLIDER_CONTROL_TYPE_INT; diff --git a/xbmc/input/ButtonTranslator.cpp b/xbmc/input/ButtonTranslator.cpp index f4293f73d7d6e..8bc1bac9fbc36 100644 --- a/xbmc/input/ButtonTranslator.cpp +++ b/xbmc/input/ButtonTranslator.cpp @@ -682,8 +682,8 @@ bool CButtonTranslator::LoadLircMap(const CStdString &lircmapPath) if (szRemote) { TiXmlAttribute* pAttr = pRemote->ToElement()->FirstAttribute(); - const char* szDeviceName = pAttr->Value(); - MapRemote(pRemote, szDeviceName); + if (pAttr) + MapRemote(pRemote, pAttr->Value()); } } pRemote = pRemote->NextSibling(); From 2eecefdb15d919da188b31b316a311002e2c8819 Mon Sep 17 00:00:00 2001 From: Jonathan Marshall Date: Tue, 1 Jul 2014 13:16:39 +1200 Subject: [PATCH 04/13] [addons] make the 'type' attribute of addon settings consistently compared case-sensitive --- xbmc/addons/AddonCallbacksAddon.cpp | 23 +++---- xbmc/addons/AddonDll.h | 35 +++++------ xbmc/addons/GUIDialogAddonSettings.cpp | 84 +++++++++++++------------- 3 files changed, 72 insertions(+), 70 deletions(-) diff --git a/xbmc/addons/AddonCallbacksAddon.cpp b/xbmc/addons/AddonCallbacksAddon.cpp index 41fc8a676daff..99256d4afe79a 100644 --- a/xbmc/addons/AddonCallbacksAddon.cpp +++ b/xbmc/addons/AddonCallbacksAddon.cpp @@ -31,6 +31,7 @@ #include "network/Network.h" #include "utils/CharsetConverter.h" #include "utils/StringUtils.h" +#include "utils/XMLUtils.h" #include "cores/dvdplayer/DVDCodecs/DVDCodecs.h" using namespace XFILE; @@ -197,31 +198,31 @@ bool CAddonCallbacksAddon::GetAddonSetting(void *addonData, const char *strSetti while (setting) { const char *id = setting->Attribute("id"); - const char *type = setting->Attribute("type"); + const std::string type = XMLUtils::GetAttribute(setting, "type"); - if (id && strcmpi(id, strSettingName) == 0 && type) + if (id && strcmpi(id, strSettingName) == 0 && !type.empty()) { - if (strcmpi(type, "text") == 0 || strcmpi(type, "ipaddress") == 0 || - strcmpi(type, "folder") == 0 || strcmpi(type, "action") == 0 || - strcmpi(type, "music") == 0 || strcmpi(type, "pictures") == 0 || - strcmpi(type, "folder") == 0 || strcmpi(type, "programs") == 0 || - strcmpi(type, "file") == 0 || strcmpi(type, "fileenum") == 0) + if (type == "text" || type == "ipaddress" || + type == "folder" || type == "action" || + type == "music" || type == "pictures" || + type == "folder" || type == "programs" || + type == "file" || type == "fileenum") { strcpy((char*) settingValue, addonHelper->m_addon->GetSetting(id).c_str()); return true; } - else if (strcmpi(type, "number") == 0 || strcmpi(type, "enum") == 0 || - strcmpi(type, "labelenum") == 0) + else if (type == "number" || type == "enum" || + type == "labelenum") { *(int*) settingValue = (int) atoi(addonHelper->m_addon->GetSetting(id)); return true; } - else if (strcmpi(type, "bool") == 0) + else if (type == "bool") { *(bool*) settingValue = (bool) (addonHelper->m_addon->GetSetting(id) == "true" ? true : false); return true; } - else if (strcmpi(type, "slider") == 0) + else if (type == "slider") { const char *option = setting->Attribute("option"); if (option && strcmpi(option, "int") == 0) diff --git a/xbmc/addons/AddonDll.h b/xbmc/addons/AddonDll.h index 2418377788e8c..5b082f37754ed 100644 --- a/xbmc/addons/AddonDll.h +++ b/xbmc/addons/AddonDll.h @@ -32,6 +32,7 @@ #include "utils/log.h" #include "interfaces/IAnnouncer.h" #include "interfaces/AnnouncementManager.h" +#include "utils/XMLUtils.h" using namespace XFILE; @@ -476,39 +477,39 @@ ADDON_STATUS CAddonDll::TransferSettings() { ADDON_STATUS status = ADDON_STATUS_OK; const char *id = setting->Attribute("id"); - const char *type = setting->Attribute("type"); + const std::string type = XMLUtils::GetAttribute(setting, "type"); const char *option = setting->Attribute("option"); - if (id && type) + if (id && !type.empty()) { - if (strcmpi(type,"sep") == 0 || strcmpi(type,"lsep") == 0) + if (type == "sep" || type == "lsep") { /* Don't propagate separators */ } - else if (strcmpi(type, "text") == 0 || strcmpi(type, "ipaddress") == 0 || - strcmpi(type, "video") == 0 || strcmpi(type, "audio") == 0 || - strcmpi(type, "image") == 0 || strcmpi(type, "folder") == 0 || - strcmpi(type, "executable") == 0 || strcmpi(type, "file") == 0 || - strcmpi(type, "action") == 0 || strcmpi(type, "date") == 0 || - strcmpi(type, "time") == 0 || strcmpi(type, "select") == 0 || - strcmpi(type, "addon") == 0 || strcmpi(type, "labelenum") == 0 || - strcmpi(type, "fileenum") == 0 ) + else if (type == "text" || type == "ipaddress" || + type == "video" || type == "audio" || + type == "image" || type == "folder" || + type == "executable" || type == "file" || + type == "action" || type == "date" || + type == "time" || type == "select" || + type == "addon" || type == "labelenum" || + type == "fileenum" ) { status = m_pDll->SetSetting(id, (const char*) GetSetting(id).c_str()); } - else if ((strcmpi(type, "enum") == 0 || strcmpi(type,"integer") == 0) || - strcmpi(type, "labelenum") == 0 || strcmpi(type, "rangeofnum") == 0) + else if (type == "enum" || type =="integer" || + type == "labelenum" || type == "rangeofnum") { int tmp = atoi(GetSetting(id)); status = m_pDll->SetSetting(id, (int*) &tmp); } - else if (strcmpi(type, "bool") == 0) + else if (type == "bool") { bool tmp = (GetSetting(id) == "true") ? true : false; status = m_pDll->SetSetting(id, (bool*) &tmp); } - else if (strcmpi(type, "rangeofnum") == 0 || strcmpi(type, "slider") == 0 || - strcmpi(type, "number") == 0) + else if (type == "rangeofnum" || type == "slider" || + type == "number") { float tmpf = (float)atof(GetSetting(id)); int tmpi; @@ -526,7 +527,7 @@ ADDON_STATUS CAddonDll::TransferSettings() else { /* Log unknowns as an error, but go ahead and transfer the string */ - CLog::Log(LOGERROR, "Unknown setting type '%s' for %s", type, Name().c_str()); + CLog::Log(LOGERROR, "Unknown setting type '%s' for %s", type.c_str(), Name().c_str()); status = m_pDll->SetSetting(id, (const char*) GetSetting(id).c_str()); } diff --git a/xbmc/addons/GUIDialogAddonSettings.cpp b/xbmc/addons/GUIDialogAddonSettings.cpp index e4b41b3c23f96..c41a856a717fb 100644 --- a/xbmc/addons/GUIDialogAddonSettings.cpp +++ b/xbmc/addons/GUIDialogAddonSettings.cpp @@ -236,16 +236,16 @@ bool CGUIDialogAddonSettings::ShowVirtualKeyboard(int iControl) { const CGUIControl* control = GetControl(controlId); const char *id = setting->Attribute("id"); - const char *type = setting->Attribute("type"); + const std::string type = XMLUtils::GetAttribute(setting, "type"); if (control && control->GetControlType() == CGUIControl::GUICONTROL_BUTTON && - id && type) + id && !type.empty()) { const char *option = setting->Attribute("option"); const char *source = setting->Attribute("source"); CStdString value = m_buttonValues[id]; CStdString label = GetString(setting->Attribute("label")); - if (strcmp(type, "text") == 0) + if (type == "text") { // get any options bool bHidden = false; @@ -273,15 +273,15 @@ bool CGUIDialogAddonSettings::ShowVirtualKeyboard(int iControl) value = CURL::Encode(value); } } - else if (strcmp(type, "number") == 0 && CGUIDialogNumeric::ShowAndGetNumber(value, label)) + else if (type == "number" && CGUIDialogNumeric::ShowAndGetNumber(value, label)) { ((CGUIButtonControl*) control)->SetLabel2(value); } - else if (strcmp(type, "ipaddress") == 0 && CGUIDialogNumeric::ShowAndGetIPAddress(value, label)) + else if (type == "ipaddress" && CGUIDialogNumeric::ShowAndGetIPAddress(value, label)) { ((CGUIButtonControl*) control)->SetLabel2(value); } - else if (strcmpi(type, "select") == 0) + else if (type == "select") { CGUIDialogSelect *pDlg = (CGUIDialogSelect*)g_windowManager.GetWindow(WINDOW_DIALOG_SELECT); if (pDlg) @@ -329,9 +329,9 @@ bool CGUIDialogAddonSettings::ShowVirtualKeyboard(int iControl) } } } - else if (strcmpi(type, "audio") == 0 || strcmpi(type, "video") == 0 || - strcmpi(type, "image") == 0 || strcmpi(type, "executable") == 0 || - strcmpi(type, "file") == 0 || strcmpi(type, "folder") == 0) + else if (type == "audio" || type == "video" + || type == "image" || type == "executable" + || type == "file" || type == "folder") { // setup the shares VECSOURCES *shares = NULL; @@ -352,7 +352,7 @@ bool CGUIDialogAddonSettings::ShowVirtualKeyboard(int iControl) g_mediaManager.GetLocalDrives(localShares); } - if (strcmpi(type, "folder") == 0) + if (type == "folder") { // get any options bool bWriteOnly = false; @@ -362,7 +362,7 @@ bool CGUIDialogAddonSettings::ShowVirtualKeyboard(int iControl) if (CGUIDialogFileBrowser::ShowAndGetDirectory(localShares, label, value, bWriteOnly)) ((CGUIButtonControl*) control)->SetLabel2(value); } - else if (strcmpi(type, "image") == 0) + else if (type == "image") { if (CGUIDialogFileBrowser::ShowAndGetImage(localShares, label, value)) ((CGUIButtonControl*) control)->SetLabel2(value); @@ -386,11 +386,11 @@ bool CGUIDialogAddonSettings::ShowVirtualKeyboard(int iControl) } else { - if (strcmpi(type, "video") == 0) + if (type == "video") strMask = g_advancedSettings.m_videoExtensions; - else if (strcmpi(type, "audio") == 0) + else if (type == "audio") strMask = g_advancedSettings.m_musicExtensions; - else if (strcmpi(type, "executable") == 0) + else if (type == "executable") #if defined(_WIN32_WINNT) strMask = ".exe|.bat|.cmd|.py"; #else @@ -412,7 +412,7 @@ bool CGUIDialogAddonSettings::ShowVirtualKeyboard(int iControl) ((CGUIButtonControl*) control)->SetLabel2(value); } } - else if (strcmpi(type, "action") == 0) + else if (type == "action") { CStdString action = XMLUtils::GetAttribute(setting, "action"); if (!action.empty()) @@ -425,7 +425,7 @@ bool CGUIDialogAddonSettings::ShowVirtualKeyboard(int iControl) CApplicationMessenger::Get().ExecBuiltIn(action); } } - else if (strcmp(type, "date") == 0) + else if (type == "date") { CDateTime date; if (!value.empty()) @@ -439,7 +439,7 @@ bool CGUIDialogAddonSettings::ShowVirtualKeyboard(int iControl) ((CGUIButtonControl*) control)->SetLabel2(value); } } - else if (strcmp(type, "time") == 0) + else if (type == "time") { SYSTEMTIME timedate; if (value.size() >= 5) @@ -454,7 +454,7 @@ bool CGUIDialogAddonSettings::ShowVirtualKeyboard(int iControl) ((CGUIButtonControl*) control)->SetLabel2(value); } } - else if (strcmp(type, "addon") == 0) + else if (type == "addon") { const char *strType = setting->Attribute("addontype"); if (strType) @@ -506,7 +506,7 @@ void CGUIDialogAddonSettings::UpdateFromControls() while (setting) { const std::string id = XMLUtils::GetAttribute(setting, "id"); - const char *type = setting->Attribute("type"); + const std::string type = XMLUtils::GetAttribute(setting, "type"); const CGUIControl* control = GetControl(controlID++); if (control) @@ -521,7 +521,7 @@ void CGUIDialogAddonSettings::UpdateFromControls() value = ((CGUIRadioButtonControl*) control)->IsSelected() ? "true" : "false"; break; case CGUIControl::GUICONTROL_SPINEX: - if (strcmpi(type, "fileenum") == 0 || strcmpi(type, "labelenum") == 0) + if (type == "fileenum" || type == "labelenum") value = ((CGUISpinControlEx*) control)->GetLabel(); else value = StringUtils::Format("%i", ((CGUISpinControlEx*) control)->GetValue()); @@ -666,7 +666,7 @@ void CGUIDialogAddonSettings::CreateControls() const TiXmlElement *setting = GetFirstSetting(); while (setting) { - const char *type = setting->Attribute("type"); + const std::string type = XMLUtils::GetAttribute(setting, "type"); const char *id = setting->Attribute("id"); CStdString values; if (setting->Attribute("values")) @@ -688,16 +688,16 @@ void CGUIDialogAddonSettings::CreateControls() if (sort && (strcmp(sort, "yes") == 0)) bSort=true; - if (id && type) + if (id && !type.empty()) { bool isAddonSetting = false; - if (strcmpi(type, "text") == 0 || strcmpi(type, "ipaddress") == 0 || - strcmpi(type, "number") == 0 ||strcmpi(type, "video") == 0 || - strcmpi(type, "audio") == 0 || strcmpi(type, "image") == 0 || - strcmpi(type, "folder") == 0 || strcmpi(type, "executable") == 0 || - strcmpi(type, "file") == 0 || strcmpi(type, "action") == 0 || - strcmpi(type, "date") == 0 || strcmpi(type, "time") == 0 || - strcmpi(type, "select") == 0 || (isAddonSetting = strcmpi(type, "addon") == 0)) + if (type == "text" || type == "ipaddress" + || type == "number" || type == "video" + || type == "audio" || type == "image" + || type == "folder" || type == "executable" + || type == "file" || type == "action" + || type == "date" || type == "time" + || type == "select" || (isAddonSetting = type == "addon")) { pControl = new CGUIButtonControl(*pOriginalButton); if (!pControl) return; @@ -720,7 +720,7 @@ void CGUIDialogAddonSettings::CreateControls() { if (isAddonSetting) ((CGUIButtonControl *)pControl)->SetLabel2(GetAddonNames(value)); - else if (strcmpi(type, "select") == 0 && !lvalues.empty()) + else if (type == "select" && !lvalues.empty()) { vector valuesVec = StringUtils::Split(lvalues, "|"); int selected = atoi(value.c_str()); @@ -739,14 +739,14 @@ void CGUIDialogAddonSettings::CreateControls() else ((CGUIButtonControl *)pControl)->SetLabel2(defaultValue); } - else if (strcmpi(type, "bool") == 0) + else if (type == "bool") { pControl = new CGUIRadioButtonControl(*pOriginalRadioButton); if (!pControl) return; ((CGUIRadioButtonControl *)pControl)->SetLabel(label); ((CGUIRadioButtonControl *)pControl)->SetSelected(m_settings[id] == "true"); } - else if (strcmpi(type, "enum") == 0 || strcmpi(type, "labelenum") == 0) + else if (type == "enum" || type == "labelenum") { vector valuesVec; vector entryVec; @@ -770,7 +770,7 @@ void CGUIDialogAddonSettings::CreateControls() if (!entries.empty()) StringUtils::Tokenize(entries, entryVec, "|"); - if(bSort && strcmpi(type, "labelenum") == 0) + if(bSort && type == "labelenum") std::sort(valuesVec.begin(), valuesVec.end(), sortstringbyname()); for (unsigned int i = 0; i < valuesVec.size(); i++) @@ -788,7 +788,7 @@ void CGUIDialogAddonSettings::CreateControls() else ((CGUISpinControlEx *)pControl)->AddLabel(valuesVec[i], iAdd); } - if (strcmpi(type, "labelenum") == 0) + if (type == "labelenum") { // need to run through all our settings and find the one that matches ((CGUISpinControlEx*) pControl)->SetValueFromLabel(m_settings[id]); } @@ -796,7 +796,7 @@ void CGUIDialogAddonSettings::CreateControls() ((CGUISpinControlEx*) pControl)->SetValue(atoi(m_settings[id])); } - else if (strcmpi(type, "fileenum") == 0) + else if (type == "fileenum") { pControl = new CGUISpinControlEx(*pOriginalSpin); if (!pControl) return; @@ -814,7 +814,7 @@ void CGUIDialogAddonSettings::CreateControls() // Sample: // in strings.xml: %2.0f mp // creates 11 piece, text formated number labels from 0 to 100 - else if (strcmpi(type, "rangeofnum") == 0) + else if (type == "rangeofnum") { pControl = new CGUISpinControlEx(*pOriginalSpin); if (!pControl) @@ -847,7 +847,7 @@ void CGUIDialogAddonSettings::CreateControls() } // Sample: // to make ints from 5-60 with 5 steps - else if (strcmpi(type, "slider") == 0) + else if (type == "slider") { pControl = new CGUISettingsSliderControl(*pOriginalSlider); if (!pControl) return; @@ -884,13 +884,13 @@ void CGUIDialogAddonSettings::CreateControls() ((CGUISettingsSliderControl *)pControl)->SetFloatInterval(fInc); ((CGUISettingsSliderControl *)pControl)->SetFloatValue((float)atof(m_settings[id])); } - else if (strcmpi(type, "lsep") == 0) + else if (type == "lsep") { pControl = new CGUILabelControl(*pOriginalLabel); if (pControl) ((CGUILabelControl *)pControl)->SetLabel(label); } - else if (strcmpi(type, "sep") == 0) + else if (type == "sep") pControl = new CGUIImage(*pOriginalImage); } @@ -1123,15 +1123,15 @@ void CGUIDialogAddonSettings::SetDefaultSettings() while (setting) { const char *id = setting->Attribute("id"); - const char *type = setting->Attribute("type"); + const std::string type = XMLUtils::GetAttribute(setting, "type"); const char *value = setting->Attribute("default"); if (id) { if (value) m_settings[id] = value; - else if (type && 0 == strcmpi(type, "bool")) + else if (type == "bool") m_settings[id] = "false"; - else if (type && (0 == strcmpi(type, "slider") || 0 == strcmpi(type, "enum"))) + else if (type == "slider" || type == "enum") m_settings[id] = "0"; else m_settings[id] = ""; From fee8d45e1d4be813b25339be1ec6569962f95d2f Mon Sep 17 00:00:00 2001 From: Jonathan Marshall Date: Wed, 2 Jul 2014 08:06:38 +1200 Subject: [PATCH 05/13] [addons] read the id attribute into a std::string where comparison is made case-sensitive --- xbmc/addons/AddonCallbacksAddon.cpp | 4 ++-- xbmc/addons/GUIDialogAddonSettings.cpp | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/xbmc/addons/AddonCallbacksAddon.cpp b/xbmc/addons/AddonCallbacksAddon.cpp index 99256d4afe79a..418b051c7b163 100644 --- a/xbmc/addons/AddonCallbacksAddon.cpp +++ b/xbmc/addons/AddonCallbacksAddon.cpp @@ -197,10 +197,10 @@ bool CAddonCallbacksAddon::GetAddonSetting(void *addonData, const char *strSetti const TiXmlElement *setting = category->FirstChildElement("setting"); while (setting) { - const char *id = setting->Attribute("id"); + const std::string id = XMLUtils::GetAttribute(setting, "id"); const std::string type = XMLUtils::GetAttribute(setting, "type"); - if (id && strcmpi(id, strSettingName) == 0 && !type.empty()) + if (id == strSettingName && !type.empty()) { if (type == "text" || type == "ipaddress" || type == "folder" || type == "action" || diff --git a/xbmc/addons/GUIDialogAddonSettings.cpp b/xbmc/addons/GUIDialogAddonSettings.cpp index c41a856a717fb..66d7d1c6d1e7a 100644 --- a/xbmc/addons/GUIDialogAddonSettings.cpp +++ b/xbmc/addons/GUIDialogAddonSettings.cpp @@ -235,10 +235,10 @@ bool CGUIDialogAddonSettings::ShowVirtualKeyboard(int iControl) if (controlId == iControl) { const CGUIControl* control = GetControl(controlId); - const char *id = setting->Attribute("id"); + const std::string id = XMLUtils::GetAttribute(setting, "id"); const std::string type = XMLUtils::GetAttribute(setting, "type"); if (control && control->GetControlType() == CGUIControl::GUICONTROL_BUTTON && - id && !type.empty()) + !id.empty() && !type.empty()) { const char *option = setting->Attribute("option"); const char *source = setting->Attribute("source"); @@ -622,8 +622,8 @@ void CGUIDialogAddonSettings::CreateSections() const TiXmlElement *setting = category->FirstChildElement("setting"); while (setting) { - const char *id = setting->Attribute("id"); - if (id) + const std::string id = XMLUtils::GetAttribute(setting, "id"); + if (!id.empty()) m_settings[id] = m_addon->GetSetting(id); setting = setting->NextSiblingElement("setting"); } From ca295a1f7f2ff8e741653bd77b86a3022094443d Mon Sep 17 00:00:00 2001 From: Jonathan Marshall Date: Tue, 1 Jul 2014 13:26:39 +1200 Subject: [PATCH 06/13] [xml] use GetAttribute() where it makes the code clearer (and where used enough to warrant the include) --- xbmc/addons/GUIDialogAddonSettings.cpp | 40 +++++++++----------------- xbmc/filesystem/RSSDirectory.cpp | 8 ++---- xbmc/guilib/GUIControlFactory.cpp | 7 ++--- xbmc/peripherals/Peripherals.cpp | 14 ++++----- xbmc/settings/AdvancedSettings.cpp | 9 ++---- xbmc/utils/Fanart.cpp | 3 +- xbmc/utils/ScraperUrl.cpp | 13 +++------ xbmc/utils/TuxBoxUtil.cpp | 11 +++---- 8 files changed, 34 insertions(+), 71 deletions(-) diff --git a/xbmc/addons/GUIDialogAddonSettings.cpp b/xbmc/addons/GUIDialogAddonSettings.cpp index 66d7d1c6d1e7a..ffb5c6d87e409 100644 --- a/xbmc/addons/GUIDialogAddonSettings.cpp +++ b/xbmc/addons/GUIDialogAddonSettings.cpp @@ -667,28 +667,16 @@ void CGUIDialogAddonSettings::CreateControls() while (setting) { const std::string type = XMLUtils::GetAttribute(setting, "type"); - const char *id = setting->Attribute("id"); - CStdString values; - if (setting->Attribute("values")) - values = setting->Attribute("values"); - CStdString lvalues; - if (setting->Attribute("lvalues")) - lvalues = setting->Attribute("lvalues"); - CStdString entries; - if (setting->Attribute("entries")) - entries = setting->Attribute("entries"); - CStdString defaultValue; - if (setting->Attribute("default")) - defaultValue= setting->Attribute("default"); + const std::string id = XMLUtils::GetAttribute(setting, "id"); + const std::string values = XMLUtils::GetAttribute(setting, "values"); + const std::string lvalues = XMLUtils::GetAttribute(setting, "lvalues"); + const std::string entries = XMLUtils::GetAttribute(setting, "entries"); + const std::string defaultValue = XMLUtils::GetAttribute(setting, "default"); const char *subsetting = setting->Attribute("subsetting"); CStdString label = GetString(setting->Attribute("label"), subsetting && 0 == strcmpi(subsetting, "true")); - bool bSort=false; - const char *sort = setting->Attribute("sort"); - if (sort && (strcmp(sort, "yes") == 0)) - bSort=true; - - if (id && !type.empty()) + bool bSort = XMLUtils::GetAttribute(setting, "sort") == "yes"; + if (!id.empty() && !type.empty()) { bool isAddonSetting = false; if (type == "text" || type == "ipaddress" @@ -702,15 +690,15 @@ void CGUIDialogAddonSettings::CreateControls() pControl = new CGUIButtonControl(*pOriginalButton); if (!pControl) return; ((CGUIButtonControl *)pControl)->SetLabel(label); - if (id) + if (!id.empty()) { CStdString value = m_settings[id]; m_buttonValues[id] = value; // get any option to test for hidden - const char *option = setting->Attribute("option"); - if (option && (strstr(option, "urlencoded"))) + const std::string option = XMLUtils::GetAttribute(setting, "option"); + if (option == "urlencoded") value = CURL::Decode(value); - if (option && (strstr(option, "hidden"))) + else if (option == "hidden") { CStdString hiddenText; hiddenText.append(value.size(), L'*'); @@ -757,7 +745,7 @@ void CGUIDialogAddonSettings::CreateControls() if (!lvalues.empty()) StringUtils::Tokenize(lvalues, valuesVec, "|"); - else if (values.Equals("$HOURS")) + else if (StringUtils::EqualsNoCase(values, "$HOURS")) { for (unsigned int i = 0; i < 24; i++) { @@ -1122,10 +1110,10 @@ void CGUIDialogAddonSettings::SetDefaultSettings() const TiXmlElement *setting = category->FirstChildElement("setting"); while (setting) { - const char *id = setting->Attribute("id"); + const std::string id = XMLUtils::GetAttribute(setting, "id"); const std::string type = XMLUtils::GetAttribute(setting, "type"); const char *value = setting->Attribute("default"); - if (id) + if (!id.empty()) { if (value) m_settings[id] = value; diff --git a/xbmc/filesystem/RSSDirectory.cpp b/xbmc/filesystem/RSSDirectory.cpp index c2763c23fa4d1..734a3950b37b9 100644 --- a/xbmc/filesystem/RSSDirectory.cpp +++ b/xbmc/filesystem/RSSDirectory.cpp @@ -278,16 +278,12 @@ static void ParseItemRSS(CFileItem* item, SResources& resources, TiXmlElement* i } else if(name == "enclosure") { - const char * url = item_child->Attribute("url"); - const char * type = item_child->Attribute("type"); const char * len = item_child->Attribute("length"); SResource res; res.tag = "rss:enclosure"; - if(url) - res.path = url; - if(type) - res.mime = type; + res.path = XMLUtils::GetAttribute(item_child, "url"); + res.mime = XMLUtils::GetAttribute(item_child, "type"); if(len) res.size = _atoi64(len); diff --git a/xbmc/guilib/GUIControlFactory.cpp b/xbmc/guilib/GUIControlFactory.cpp index e8c0db024522e..2fea2255b1545 100644 --- a/xbmc/guilib/GUIControlFactory.cpp +++ b/xbmc/guilib/GUIControlFactory.cpp @@ -658,11 +658,8 @@ bool CGUIControlFactory::GetString(const TiXmlNode* pRootNode, const char *strTa CStdString CGUIControlFactory::GetType(const TiXmlElement *pControlNode) { - CStdString type; - const char *szType = pControlNode->Attribute("type"); - if (szType) - type = szType; - else // backward compatibility - not desired + CStdString type = XMLUtils::GetAttribute(pControlNode, "type"); + if (type.empty()) // backward compatibility - not desired XMLUtils::GetString(pControlNode, "type", type); return type; } diff --git a/xbmc/peripherals/Peripherals.cpp b/xbmc/peripherals/Peripherals.cpp index d75542924c13e..557ee351f44e1 100644 --- a/xbmc/peripherals/Peripherals.cpp +++ b/xbmc/peripherals/Peripherals.cpp @@ -430,7 +430,7 @@ bool CPeripherals::LoadMappings(void) PeripheralID id; PeripheralDeviceMapping mapping; - mapping.m_strDeviceName = currentNode->Attribute("name") ? CStdString(currentNode->Attribute("name")) : StringUtils::EmptyString; + mapping.m_strDeviceName = XMLUtils::GetAttribute(currentNode, "name"); // If there is no vendor_product attribute ignore this entry if (currentNode->Attribute("vendor_product")) @@ -478,16 +478,12 @@ void CPeripherals::GetSettingsFromMappingsFile(TiXmlElement *xmlNode, mapAttribute("label") ? atoi(currentNode->Attribute("label")) : -1; - bool bConfigurable = (!currentNode->Attribute("configurable") || - strcmp(currentNode->Attribute("configurable"), "") == 0 || - (strcmp(currentNode->Attribute("configurable"), "no") != 0 && - strcmp(currentNode->Attribute("configurable"), "false") != 0 && - strcmp(currentNode->Attribute("configurable"), "0") != 0)); + const std::string config = XMLUtils::GetAttribute(currentNode, "configurable"); + bool bConfigurable = (config.empty() || (config != "no" && config != "false" && config != "0")); if (strSettingsType.Equals("bool")) { - bool bValue = (strcmp(currentNode->Attribute("value"), "no") != 0 && - strcmp(currentNode->Attribute("value"), "false") != 0 && - strcmp(currentNode->Attribute("value"), "0") != 0); + const std::string value = XMLUtils::GetAttribute(currentNode, "value"); + bool bValue = (value != "no" && value != "false" && value != "0"); setting = new CSettingBool(strKey, iLabelId, bValue); } else if (strSettingsType.Equals("int")) diff --git a/xbmc/settings/AdvancedSettings.cpp b/xbmc/settings/AdvancedSettings.cpp index 6e4e2b8f1f937..d28d6064c04a1 100644 --- a/xbmc/settings/AdvancedSettings.cpp +++ b/xbmc/settings/AdvancedSettings.cpp @@ -527,13 +527,8 @@ void CAdvancedSettings::ParseSettingsFile(const CStdString &file) TiXmlElement* pKaraokeBackground = pElement->FirstChildElement("defaultbackground"); if (pKaraokeBackground) { - const char* attr = pKaraokeBackground->Attribute("type"); - if ( attr ) - m_karaokeDefaultBackgroundType = attr; - - attr = pKaraokeBackground->Attribute("path"); - if ( attr ) - m_karaokeDefaultBackgroundFilePath = attr; + pKaraokeBackground->QueryStringAttribute("type", &m_karaokeDefaultBackgroundType); + pKaraokeBackground->QueryStringAttribute("path", &m_karaokeDefaultBackgroundFilePath); } } diff --git a/xbmc/utils/Fanart.cpp b/xbmc/utils/Fanart.cpp index e713b59e0406b..58eecaabce3ea 100644 --- a/xbmc/utils/Fanart.cpp +++ b/xbmc/utils/Fanart.cpp @@ -71,8 +71,7 @@ bool CFanart::Unpack() if (url.empty()) { data.strImage = fanartThumb->GetText(); - if (fanartThumb->Attribute("preview")) - data.strPreview = fanartThumb->Attribute("preview"); + data.strPreview = XMLUtils::GetAttribute(fanartThumb, "preview"); } else { diff --git a/xbmc/utils/ScraperUrl.cpp b/xbmc/utils/ScraperUrl.cpp index 907d0f41c354f..b7eb719f00ea4 100644 --- a/xbmc/utils/ScraperUrl.cpp +++ b/xbmc/utils/ScraperUrl.cpp @@ -30,6 +30,7 @@ #include "filesystem/ZipFile.h" #include "URIUtils.h" #include "utils/XBMCTinyXML.h" +#include "utils/XMLUtils.h" #include "utils/Mime.h" #include @@ -84,9 +85,7 @@ bool CScraperUrl::ParseElement(const TiXmlElement* element) SUrlEntry url; url.m_url = element->FirstChild()->Value(); - const char* pSpoof = element->Attribute("spoof"); - if (pSpoof) - url.m_spoof = pSpoof; + url.m_spoof = XMLUtils::GetAttribute(element, "spoof"); const char* szPost=element->Attribute("post"); if (szPost && stricmp(szPost,"yes") == 0) url.m_post = true; @@ -97,9 +96,7 @@ bool CScraperUrl::ParseElement(const TiXmlElement* element) url.m_isgz = true; else url.m_isgz = false; - const char* pCache = element->Attribute("cache"); - if (pCache) - url.m_cache = pCache; + url.m_cache = XMLUtils::GetAttribute(element, "cache"); const char* szType = element->Attribute("type"); url.m_type = URL_TYPE_GENERAL; @@ -111,9 +108,7 @@ bool CScraperUrl::ParseElement(const TiXmlElement* element) if (szSeason) url.m_season = atoi(szSeason); } - const char *aspect = element->Attribute("aspect"); - if (aspect) - url.m_aspect = aspect; + url.m_aspect = XMLUtils::GetAttribute(element, "aspect"); m_url.push_back(url); diff --git a/xbmc/utils/TuxBoxUtil.cpp b/xbmc/utils/TuxBoxUtil.cpp index a675b53a9d597..826fb0b1a9089 100644 --- a/xbmc/utils/TuxBoxUtil.cpp +++ b/xbmc/utils/TuxBoxUtil.cpp @@ -40,6 +40,7 @@ #include "guilib/LocalizeStrings.h" #include "StringUtils.h" #include "utils/XBMCTinyXML.h" +#include "utils/XMLUtils.h" #include "log.h" using namespace XFILE; @@ -1516,7 +1517,6 @@ CStdString CTuxBoxUtil::GetPicon(CStdString strServiceName) else { CStdString piconXML, piconPath, defaultPng; - CStdString strName, strPng; piconPath = "special://xbmc/userdata/PictureIcon/Picon/"; defaultPng = piconPath+"tuxbox.png"; piconXML = "special://xbmc/userdata/PictureIcon/picon.xml"; @@ -1543,15 +1543,12 @@ CStdString CTuxBoxUtil::GetPicon(CStdString strServiceName) pService = pServices->FirstChildElement("service"); while(pService) { - if(pService->Attribute("name")) - strName = StringUtils::Format("%s", pService->Attribute("name")); - - if(pService->Attribute("png")) - strPng = StringUtils::Format("%s", pService->Attribute("png")); + CStdString strName = XMLUtils::GetAttribute(pService, "name"); + CStdString strPng = XMLUtils::GetAttribute(pService, "png"); if(strName.Equals(strServiceName)) { - strPng = StringUtils::Format("%s%s", piconPath.c_str(), strPng.c_str()); + strPng = piconPath + strPng; StringUtils::ToLower(strPng); CLog::Log(LOGDEBUG, "%s %s: Path is: %s", __FUNCTION__,strServiceName.c_str(), strPng.c_str()); return strPng; From 29e767be6fe271abdbb57b1cf42139ba63e52528 Mon Sep 17 00:00:00 2001 From: Jonathan Marshall Date: Tue, 1 Jul 2014 13:28:02 +1200 Subject: [PATCH 07/13] [xml] use TiXmlNode::Attribute(foo, int|double) method where it tidies things up --- xbmc/addons/GUIDialogAddonSettings.cpp | 14 ++++++-------- xbmc/filesystem/RSSDirectory.cpp | 12 ++++-------- 2 files changed, 10 insertions(+), 16 deletions(-) diff --git a/xbmc/addons/GUIDialogAddonSettings.cpp b/xbmc/addons/GUIDialogAddonSettings.cpp index ffb5c6d87e409..3c81b94e7106f 100644 --- a/xbmc/addons/GUIDialogAddonSettings.cpp +++ b/xbmc/addons/GUIDialogAddonSettings.cpp @@ -810,15 +810,13 @@ void CGUIDialogAddonSettings::CreateControls() ((CGUISpinControlEx *)pControl)->SetText(label); ((CGUISpinControlEx *)pControl)->SetFloatValue(1.0f); - double rangestart = 0; - if (setting->Attribute("rangestart")) - rangestart = atof(setting->Attribute("rangestart")); - double rangeend = 1; - if (setting->Attribute("rangeend")) - rangeend = atof(setting->Attribute("rangeend")); + double rangestart = 0, rangeend = 1; + setting->Attribute("rangestart", &rangestart); + setting->Attribute("rangeend", &rangeend); + int elements = 2; - if (setting->Attribute("elements")) - elements = atoi(setting->Attribute("elements")); + setting->Attribute("elements", &elements); + CStdString valueformat; if (setting->Attribute("valueformat")) valueformat = m_addon->GetString(atoi(setting->Attribute("valueformat"))); diff --git a/xbmc/filesystem/RSSDirectory.cpp b/xbmc/filesystem/RSSDirectory.cpp index 734a3950b37b9..e681a7e2b3cb3 100644 --- a/xbmc/filesystem/RSSDirectory.cpp +++ b/xbmc/filesystem/RSSDirectory.cpp @@ -124,14 +124,10 @@ static void ParseItemMRSS(CFileItem* item, SResources& resources, TiXmlElement* res.tag = "media:content"; res.mime = XMLUtils::GetAttribute(item_child, "type"); res.path = XMLUtils::GetAttribute(item_child, "url"); - if(item_child->Attribute("width")) - res.width = atoi(item_child->Attribute("width")); - if(item_child->Attribute("height")) - res.height = atoi(item_child->Attribute("height")); - if(item_child->Attribute("bitrate")) - res.bitrate = atoi(item_child->Attribute("bitrate")); - if(item_child->Attribute("duration")) - res.duration = atoi(item_child->Attribute("duration")); + item_child->Attribute("width", &res.width); + item_child->Attribute("height", &res.height); + item_child->Attribute("bitrate", &res.bitrate); + item_child->Attribute("duration", &res.duration); if(item_child->Attribute("fileSize")) res.size = _atoi64(item_child->Attribute("fileSize")); From a42ee0d9a7dd7ce90fe72fe9a348d6c39f1ef23f Mon Sep 17 00:00:00 2001 From: Jonathan Marshall Date: Tue, 1 Jul 2014 13:28:38 +1200 Subject: [PATCH 08/13] [xml] read attributes into const char* where they're re-used immediately --- xbmc/addons/GUIDialogAddonSettings.cpp | 10 ++++++---- xbmc/filesystem/SlingboxFile.cpp | 4 ++-- xbmc/interfaces/info/SkinVariable.cpp | 5 +++-- 3 files changed, 11 insertions(+), 8 deletions(-) diff --git a/xbmc/addons/GUIDialogAddonSettings.cpp b/xbmc/addons/GUIDialogAddonSettings.cpp index 3c81b94e7106f..b71ac861ba3fb 100644 --- a/xbmc/addons/GUIDialogAddonSettings.cpp +++ b/xbmc/addons/GUIDialogAddonSettings.cpp @@ -961,13 +961,15 @@ void CGUIDialogAddonSettings::EnableControls() if (control) { // set enable status - if (setting->Attribute("enable")) - ((CGUIControl*) control)->SetEnabled(GetCondition(setting->Attribute("enable"), controlId)); + const char *enable = setting->Attribute("enable"); + if (enable) + ((CGUIControl*) control)->SetEnabled(GetCondition(enable, controlId)); else ((CGUIControl*) control)->SetEnabled(true); // set visible status - if (setting->Attribute("visible")) - ((CGUIControl*) control)->SetVisible(GetCondition(setting->Attribute("visible"), controlId)); + const char *visible = setting->Attribute("visible"); + if (visible) + ((CGUIControl*) control)->SetVisible(GetCondition(visible, controlId)); else ((CGUIControl*) control)->SetVisible(true); } diff --git a/xbmc/filesystem/SlingboxFile.cpp b/xbmc/filesystem/SlingboxFile.cpp index 8cdbc7ceed7f8..156b4ebd3ca10 100644 --- a/xbmc/filesystem/SlingboxFile.cpp +++ b/xbmc/filesystem/SlingboxFile.cpp @@ -527,8 +527,8 @@ void CSlingboxFile::LoadSettings(const CStdString& strHostname) for (pElement = pRootElement->FirstChildElement("slingbox"); pElement; pElement = pElement->NextSiblingElement("slingbox")) { - if (pElement->Attribute("hostname") == NULL || - StringUtils::EqualsNoCase(m_sSlingboxSettings.strHostname, pElement->Attribute("hostname"))) + const char *hostname = pElement->Attribute("hostname"); + if (!hostname || StringUtils::EqualsNoCase(m_sSlingboxSettings.strHostname, hostname)) { // Load setting values XMLUtils::GetInt(pElement, "width", m_sSlingboxSettings.iVideoWidth, 0, 640); diff --git a/xbmc/interfaces/info/SkinVariable.cpp b/xbmc/interfaces/info/SkinVariable.cpp index 5ca762820b2da..6316f949f296b 100644 --- a/xbmc/interfaces/info/SkinVariable.cpp +++ b/xbmc/interfaces/info/SkinVariable.cpp @@ -39,8 +39,9 @@ const CSkinVariableString* CSkinVariable::CreateFromXML(const TiXmlElement& node if (valuenode->FirstChild()) { CSkinVariableString::ConditionLabelPair pair; - if (valuenode->Attribute("condition")) - pair.m_condition = g_infoManager.Register(valuenode->Attribute("condition"), context); + const char *condition = valuenode->Attribute("condition"); + if (condition) + pair.m_condition = g_infoManager.Register(condition, context); pair.m_label = CGUIInfoLabel(valuenode->FirstChild()->Value()); tmp->m_conditionLabelPairs.push_back(pair); From 4b784397e6434cbba5a778068698a4c276c9fc78 Mon Sep 17 00:00:00 2001 From: Jonathan Marshall Date: Tue, 1 Jul 2014 13:29:26 +1200 Subject: [PATCH 09/13] [skinsettings] setting type of skin settings needn't be compared case-insensitive (we write them, we read them) --- xbmc/settings/SkinSettings.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/xbmc/settings/SkinSettings.cpp b/xbmc/settings/SkinSettings.cpp index 66a273ccce3b0..812df4ffa1b8f 100644 --- a/xbmc/settings/SkinSettings.cpp +++ b/xbmc/settings/SkinSettings.cpp @@ -210,7 +210,8 @@ bool CSkinSettings::Load(const TiXmlNode *settings) while (pChild) { std::string settingName = XMLUtils::GetAttribute(pChild, XML_ATTR_NAME); - if (pChild->Attribute("type") && StringUtils::EqualsNoCase(pChild->Attribute(XML_ATTR_TYPE), "string")) + std::string settingType = XMLUtils::GetAttribute(pChild, XML_ATTR_TYPE); + if (settingType == "string") { // string setting CSkinString string; string.name = settingName; From 6f5cd0296572b2fd067e438488f9275bb9693512 Mon Sep 17 00:00:00 2001 From: Jonathan Marshall Date: Tue, 1 Jul 2014 13:30:15 +1200 Subject: [PATCH 10/13] [addons] have the 'subsetting' attribute and '' values attribute be case-sensitive to drop some strcmpi() --- xbmc/addons/GUIDialogAddonSettings.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/xbmc/addons/GUIDialogAddonSettings.cpp b/xbmc/addons/GUIDialogAddonSettings.cpp index b71ac861ba3fb..22fb3cd3a064d 100644 --- a/xbmc/addons/GUIDialogAddonSettings.cpp +++ b/xbmc/addons/GUIDialogAddonSettings.cpp @@ -672,8 +672,8 @@ void CGUIDialogAddonSettings::CreateControls() const std::string lvalues = XMLUtils::GetAttribute(setting, "lvalues"); const std::string entries = XMLUtils::GetAttribute(setting, "entries"); const std::string defaultValue = XMLUtils::GetAttribute(setting, "default"); - const char *subsetting = setting->Attribute("subsetting"); - CStdString label = GetString(setting->Attribute("label"), subsetting && 0 == strcmpi(subsetting, "true")); + const std::string subsetting = XMLUtils::GetAttribute(setting, "subsetting"); + const std::string label = GetString(setting->Attribute("label"), subsetting == "true"); bool bSort = XMLUtils::GetAttribute(setting, "sort") == "yes"; if (!id.empty() && !type.empty()) @@ -745,7 +745,7 @@ void CGUIDialogAddonSettings::CreateControls() if (!lvalues.empty()) StringUtils::Tokenize(lvalues, valuesVec, "|"); - else if (StringUtils::EqualsNoCase(values, "$HOURS")) + else if (values == "$HOURS") { for (unsigned int i = 0; i < 24; i++) { From d2a666df15b66284718df393b415f6eab07c2934 Mon Sep 17 00:00:00 2001 From: Jonathan Marshall Date: Tue, 1 Jul 2014 13:31:04 +1200 Subject: [PATCH 11/13] [settings] make the advancedsetting 'bydate' attribute for tvshow regexps case-sensitive (i.e. 'true') --- xbmc/settings/AdvancedSettings.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/xbmc/settings/AdvancedSettings.cpp b/xbmc/settings/AdvancedSettings.cpp index d28d6064c04a1..59b4d9e89a0c6 100644 --- a/xbmc/settings/AdvancedSettings.cpp +++ b/xbmc/settings/AdvancedSettings.cpp @@ -1234,7 +1234,7 @@ void CAdvancedSettings::GetCustomTVRegexps(TiXmlElement *pRootElement, SETTINGS_ if (pRegExp->ToElement()) { CStdString byDate = XMLUtils::GetAttribute(pRegExp->ToElement(), "bydate"); - if (stricmp(byDate, "true") == 0) + if (byDate == "true") { bByDate = true; } From 4b6c3b95d8e4dbcc1a8f606da389aed7f38a9275 Mon Sep 17 00:00:00 2001 From: Jonathan Marshall Date: Wed, 2 Jul 2014 08:10:02 +1200 Subject: [PATCH 12/13] [addons] drop duplicate if clause --- xbmc/addons/AddonCallbacksAddon.cpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/xbmc/addons/AddonCallbacksAddon.cpp b/xbmc/addons/AddonCallbacksAddon.cpp index 418b051c7b163..7ad5dfafcc826 100644 --- a/xbmc/addons/AddonCallbacksAddon.cpp +++ b/xbmc/addons/AddonCallbacksAddon.cpp @@ -202,11 +202,11 @@ bool CAddonCallbacksAddon::GetAddonSetting(void *addonData, const char *strSetti if (id == strSettingName && !type.empty()) { - if (type == "text" || type == "ipaddress" || - type == "folder" || type == "action" || - type == "music" || type == "pictures" || - type == "folder" || type == "programs" || - type == "file" || type == "fileenum") + if (type == "text" || type == "ipaddress" || + type == "folder" || type == "action" || + type == "music" || type == "pictures" || + type == "programs" || type == "fileenum" || + type == "file") { strcpy((char*) settingValue, addonHelper->m_addon->GetSetting(id).c_str()); return true; From a3629429b68c8b295547a1fed8d9554fb9a46396 Mon Sep 17 00:00:00 2001 From: Jonathan Marshall Date: Wed, 2 Jul 2014 08:13:57 +1200 Subject: [PATCH 13/13] [cosmetic] align assignment block --- xbmc/addons/GUIDialogAddonSettings.cpp | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/xbmc/addons/GUIDialogAddonSettings.cpp b/xbmc/addons/GUIDialogAddonSettings.cpp index 22fb3cd3a064d..2228108d61ee1 100644 --- a/xbmc/addons/GUIDialogAddonSettings.cpp +++ b/xbmc/addons/GUIDialogAddonSettings.cpp @@ -666,14 +666,14 @@ void CGUIDialogAddonSettings::CreateControls() const TiXmlElement *setting = GetFirstSetting(); while (setting) { - const std::string type = XMLUtils::GetAttribute(setting, "type"); - const std::string id = XMLUtils::GetAttribute(setting, "id"); - const std::string values = XMLUtils::GetAttribute(setting, "values"); - const std::string lvalues = XMLUtils::GetAttribute(setting, "lvalues"); - const std::string entries = XMLUtils::GetAttribute(setting, "entries"); - const std::string defaultValue = XMLUtils::GetAttribute(setting, "default"); + const std::string type = XMLUtils::GetAttribute(setting, "type"); + const std::string id = XMLUtils::GetAttribute(setting, "id"); + const std::string values = XMLUtils::GetAttribute(setting, "values"); + const std::string lvalues = XMLUtils::GetAttribute(setting, "lvalues"); + const std::string entries = XMLUtils::GetAttribute(setting, "entries"); + const std::string defaultVal = XMLUtils::GetAttribute(setting, "default"); const std::string subsetting = XMLUtils::GetAttribute(setting, "subsetting"); - const std::string label = GetString(setting->Attribute("label"), subsetting == "true"); + const std::string label = GetString(setting->Attribute("label"), subsetting == "true"); bool bSort = XMLUtils::GetAttribute(setting, "sort") == "yes"; if (!id.empty() && !type.empty()) @@ -725,7 +725,7 @@ void CGUIDialogAddonSettings::CreateControls() } } else - ((CGUIButtonControl *)pControl)->SetLabel2(defaultValue); + ((CGUIButtonControl *)pControl)->SetLabel2(defaultVal); } else if (type == "bool") {