From fadb863eb79b370a65b0982611f6c52f53effdf8 Mon Sep 17 00:00:00 2001
From: Julien Osman <julien.osman@csgroup.eu>
Date: Wed, 15 Jul 2020 10:50:12 +0200
Subject: [PATCH] BUG: Avoid a memory leak and avoid returning const strings

---
 .../Metadata/include/otbMetadataSupplierInterface.h    |  2 +-
 Modules/Core/Metadata/include/otbXMLMetadataSupplier.h |  4 ++--
 Modules/Core/Metadata/src/otbXMLMetadataSupplier.cxx   | 10 ++++++----
 Modules/IO/IOGDAL/include/otbGDALImageIO.h             |  2 +-
 Modules/IO/IOGDAL/src/otbGDALImageIO.cxx               |  2 +-
 5 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/Modules/Core/Metadata/include/otbMetadataSupplierInterface.h b/Modules/Core/Metadata/include/otbMetadataSupplierInterface.h
index 0e1aa8b4d7..3efc3ef8e5 100644
--- a/Modules/Core/Metadata/include/otbMetadataSupplierInterface.h
+++ b/Modules/Core/Metadata/include/otbMetadataSupplierInterface.h
@@ -77,7 +77,7 @@ public:
    * depends on the specific implementation. Returns empty string when path is not found,
    * and hasValue is set to False.
    * If band >= 0, the metadata value is looked in the specified band*/
-  virtual const std::string GetMetadataValue(const std::string path, bool& hasValue, int band=-1) const = 0;
+  virtual std::string GetMetadataValue(const std::string path, bool& hasValue, int band=-1) const = 0;
 
   bool HasValue(std::string path, int band=-1);
 
diff --git a/Modules/Core/Metadata/include/otbXMLMetadataSupplier.h b/Modules/Core/Metadata/include/otbXMLMetadataSupplier.h
index 2b78d3cc96..0547b346f1 100644
--- a/Modules/Core/Metadata/include/otbXMLMetadataSupplier.h
+++ b/Modules/Core/Metadata/include/otbXMLMetadataSupplier.h
@@ -52,7 +52,7 @@ public:
    * @param band not used
    * @return The value corresponding to path. Empty string if not found.
    */
-  const std::string GetMetadataValue(const std::string path, bool& hasValue, int band=1) const override;
+  std::string GetMetadataValue(const std::string path, bool& hasValue, int band=1) const override;
 
   /**
    * @brief Get the first metadata value corresponding to a given path
@@ -61,7 +61,7 @@ public:
    * @param hasValue True if path is found
    * @return The value corresponding to path. Empty string if not found.
    */
-  const std::string GetFirstMetadataValue(const std::string paths, bool& hasValue) const;
+  std::string GetFirstMetadataValue(const std::string paths, bool& hasValue) const;
 
   /**
    * @brief Get the metadata value corresponding to a given path
diff --git a/Modules/Core/Metadata/src/otbXMLMetadataSupplier.cxx b/Modules/Core/Metadata/src/otbXMLMetadataSupplier.cxx
index 5470ac1839..0303154a1e 100644
--- a/Modules/Core/Metadata/src/otbXMLMetadataSupplier.cxx
+++ b/Modules/Core/Metadata/src/otbXMLMetadataSupplier.cxx
@@ -36,7 +36,7 @@ XMLMetadataSupplier::XMLMetadataSupplier(const std::string & fileName)
   CPLDestroyXMLNode(psNode);
 }
 
-const std::string XMLMetadataSupplier::GetMetadataValue(const std::string path, bool& hasValue, int band) const
+std::string XMLMetadataSupplier::GetMetadataValue(const std::string path, bool& hasValue, int band) const
 {
   const char * ret = CSLFetchNameValue(m_MetadataDic, path.c_str());
   if (ret)
@@ -49,7 +49,7 @@ const std::string XMLMetadataSupplier::GetMetadataValue(const std::string path,
   return std::string(ret);
 }
 
-const std::string XMLMetadataSupplier::GetFirstMetadataValue(const std::string path, bool& hasValue) const
+std::string XMLMetadataSupplier::GetFirstMetadataValue(const std::string path, bool& hasValue) const
 {
   // Search for the  first joker
   std::size_t found = path.find("_#");
@@ -74,9 +74,11 @@ const std::string XMLMetadataSupplier::GetFirstMetadataValue(const std::string p
   if ((values != nullptr) && (values[0] != nullptr))
   {
     hasValue = true;
-    std::string ret = std::string(values[0]);
+    std::string ret = values[0];
+    ret = ret.substr(ret.find('=') + 1);
+    CSLDestroy(values);
     // Return the value part
-    return ret.substr(ret.find('=') + 1);
+    return ret;
   }
   else
   {
diff --git a/Modules/IO/IOGDAL/include/otbGDALImageIO.h b/Modules/IO/IOGDAL/include/otbGDALImageIO.h
index ccbb07123a..65a33def3e 100644
--- a/Modules/IO/IOGDAL/include/otbGDALImageIO.h
+++ b/Modules/IO/IOGDAL/include/otbGDALImageIO.h
@@ -210,7 +210,7 @@ public:
   std::vector<std::string> GetResourceFiles() const override;
 
   /** Get metadata item in GDALDataset, domain can specified as "domain/key" */
-  const std::string GetMetadataValue(const std::string path, bool& hasValue, int band = -1) const override;
+  std::string GetMetadataValue(const std::string path, bool& hasValue, int band = -1) const override;
 
   /** Set metadata item in GDALDataset, domain can specified as prefix of the
    *  path, like "domain/key"*/
diff --git a/Modules/IO/IOGDAL/src/otbGDALImageIO.cxx b/Modules/IO/IOGDAL/src/otbGDALImageIO.cxx
index 41505a8155..5dcb6b8ae1 100644
--- a/Modules/IO/IOGDAL/src/otbGDALImageIO.cxx
+++ b/Modules/IO/IOGDAL/src/otbGDALImageIO.cxx
@@ -1835,7 +1835,7 @@ std::vector<std::string> GDALImageIO::GetResourceFiles() const
   return result;
 }
 
-const std::string GDALImageIO::GetMetadataValue(const std::string path, bool& hasValue, int band) const
+std::string GDALImageIO::GetMetadataValue(const std::string path, bool& hasValue, int band) const
 {
   // detect namespace if any
   std::string domain("");
-- 
GitLab