From: Ilho Kim <ilho159.kim@samsung.com>
Date: Fri, 25 Sep 2020 07:46:55 +0000 (+0900)
Subject: Fix memory leak
X-Git-Tag: accepted/tizen/6.0/unified/20201113.014551~2
X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=467543bd672dca14cb47f5e6ac96f323a3b67e5e;p=platform%2Fcore%2Fappfw%2Fwgt-backend.git

Fix memory leak

The value created by LoadXmlDocument should be freed using xmlFreeDoc
member variables wgt_doc_ and tpk_doc_ are used in function process()
so remove that from member variables and that will be freed using unique_ptr

Change-Id: I477a1e2677f30773efaacb058bde1a5c56b688d0
Signed-off-by: Ilho Kim <ilho159.kim@samsung.com>
---

diff --git a/src/hybrid/step/pkgmgr/step_merge_xml.cc b/src/hybrid/step/pkgmgr/step_merge_xml.cc
index 529545c..8b6fbb3 100644
--- a/src/hybrid/step/pkgmgr/step_merge_xml.cc
+++ b/src/hybrid/step/pkgmgr/step_merge_xml.cc
@@ -54,24 +54,17 @@ const char kXmlXPrivilegeExpr[] = "//*[local-name()='privilege']";
 namespace hybrid {
 namespace pkgmgr {
 
-bool StepMergeXml::LoadXmlDocument(const bf::path& wgt_xml_path,
-    const bf::path& tpk_xml_path) {
+xmlDocPtr StepMergeXml::LoadXmlDocument(const bf::path& xml_path) {
   // trim blanks
   int keep_blanks = xmlKeepBlanksDefault(0);
-  wgt_doc_ = xmlParseFile(wgt_xml_path.string().c_str());
-  if (!wgt_doc_) {
-    LOG(ERROR) << "Failed to parse file: " << wgt_xml_path;
+  xmlDocPtr doc_ptr = xmlParseFile(xml_path.string().c_str());
+  if (!doc_ptr) {
+    LOG(ERROR) << "Failed to parse file: " << xml_path;
     xmlKeepBlanksDefault(keep_blanks);
-    return false;
-  }
-  tpk_doc_ = xmlParseFile(tpk_xml_path.string().c_str());
-  if (!tpk_doc_) {
-    LOG(ERROR) << "Failed to parse file: " << tpk_xml_path;
-    xmlKeepBlanksDefault(keep_blanks);
-    return false;
+    return nullptr;
   }
   xmlKeepBlanksDefault(keep_blanks);
-  return true;
+  return doc_ptr;
 }
 
 xmlNodePtr StepMergeXml::GetXmlNode(const xmlDocPtr doc,
@@ -127,8 +120,8 @@ void StepMergeXml::SetXmlNodeAttribute(xmlNodePtr node,
       reinterpret_cast<const xmlChar*>(attr_val.c_str()));
 }
 
-bool StepMergeXml::SetTpkPrivilegeType() {
-  xmlXPathContextPtr xpath_ctx = xmlXPathNewContext(tpk_doc_);
+bool StepMergeXml::SetTpkPrivilegeType(xmlDocPtr tpk_doc) {
+  xmlXPathContextPtr xpath_ctx = xmlXPathNewContext(tpk_doc);
   if (!xpath_ctx) {
     LOG(ERROR) << "Failed to create XPath context";
     return false;
@@ -152,19 +145,30 @@ ci::Step::Status StepMergeXml::process() {
   bf::path wgt_xml_path = context_->xml_path.get();
   bf::path tpk_xml_path = context_->GetPkgPath() / "tizen-manifest.xml";
 
-  if (!LoadXmlDocument(wgt_xml_path, tpk_xml_path))
+  xmlDocPtr wgt_doc = LoadXmlDocument(wgt_xml_path);
+  if (wgt_doc == nullptr)
     return Step::Status::MANIFEST_ERROR;
 
-  if (!SetTpkPrivilegeType())
+  auto wgt_doc_ptr = std::unique_ptr<std::remove_pointer<xmlDocPtr>::type,
+      decltype(xmlFreeDoc)*>(wgt_doc, xmlFreeDoc);
+
+  xmlDocPtr tpk_doc = LoadXmlDocument(tpk_xml_path);
+  if (tpk_doc == nullptr)
+    return Step::Status::MANIFEST_ERROR;
+
+  auto tpk_doc_ptr = std::unique_ptr<std::remove_pointer<xmlDocPtr>::type,
+      decltype(xmlFreeDoc)*>(tpk_doc, xmlFreeDoc);
+
+  if (!SetTpkPrivilegeType(tpk_doc))
     return Step::Status::MANIFEST_ERROR;
 
   for (auto& entry : kNeedMergeNodes) {
-    xmlNodePtr tpk_node = GetXmlNode(tpk_doc_, entry);
+    xmlNodePtr tpk_node = GetXmlNode(tpk_doc, entry);
     if (tpk_node == nullptr)
       continue;
     // TODO(s89.jang): If cannot find node from wgt doc, merge tpk node itself
     // instead of merging its child nodes.
-    xmlNodePtr wgt_node = GetXmlNode(wgt_doc_, entry);
+    xmlNodePtr wgt_node = GetXmlNode(wgt_doc, entry);
     MergeXmlNode(wgt_node, tpk_node);
   }
 
@@ -183,11 +187,11 @@ ci::Step::Status StepMergeXml::process() {
       LOG(WARNING) << "Cannot find component type!";
       continue;
     }
-    xmlNodePtr node = GetXmlNode(wgt_doc_, (*r).first, "appid", app->appid);
+    xmlNodePtr node = GetXmlNode(wgt_doc, (*r).first, "appid", app->appid);
     SetXmlNodeAttribute(node, "exec", app->exec);
   }
 
-  if (xmlSaveFormatFile(wgt_xml_path.string().c_str(), wgt_doc_, 1) == -1) {
+  if (xmlSaveFormatFile(wgt_xml_path.string().c_str(), wgt_doc, 1) == -1) {
     LOG(ERROR) << "Cannot write xml file";
     return Step::Status::MANIFEST_ERROR;
   }
diff --git a/src/hybrid/step/pkgmgr/step_merge_xml.h b/src/hybrid/step/pkgmgr/step_merge_xml.h
index a373bcf..e9e606b 100644
--- a/src/hybrid/step/pkgmgr/step_merge_xml.h
+++ b/src/hybrid/step/pkgmgr/step_merge_xml.h
@@ -20,24 +20,20 @@ class StepMergeXml : public common_installer::Step {
  public:
   using Step::Step;
   explicit StepMergeXml(common_installer::InstallerContext* context)
-      : Step(context), wgt_doc_(nullptr), tpk_doc_(nullptr) {}
+      : Step(context) {}
   Status process() override;
   Status clean() override { return Status::OK; }
   Status undo() override { return Status::OK; }
   Status precheck() override;
 
  private:
-  bool LoadXmlDocument(const boost::filesystem::path& wgt_xml_path,
-      const boost::filesystem::path& tpk_xml_path);
+  xmlDocPtr LoadXmlDocument(const boost::filesystem::path& xml_path);
   xmlNodePtr GetXmlNode(const xmlDocPtr doc, const std::string& name,
       const std::string& attr = {}, const std::string& attr_val = {});
   void MergeXmlNode(xmlNodePtr node1, xmlNodePtr node2);
   void SetXmlNodeAttribute(xmlNodePtr node, const std::string& attr,
       const std::string& attr_val);
-  bool SetTpkPrivilegeType();
-
-  xmlDocPtr wgt_doc_;
-  xmlDocPtr tpk_doc_;
+  bool SetTpkPrivilegeType(xmlDocPtr tpk_doc);
 
   STEP_NAME(MergeXml);
 };