Problem passing back values in parameter

[GoogleCodeExporter]

CXmlNode::CXmlNode()
{
   m_XmlNode = new pugi::xml_node ;
}

CXmlDocument::CXmlDocument()
{
 m_XmlDocument = new pugi::xml_document;
}

CXmlDocument::GetChildNode(const std::wstring& xpath,uint index, CXmlNode& node)
{
   CConverter m_Converter;      
   uint nodeIndex = 0;
   pugi::xpath_node_set xPathNodeSet;
   int retVal;
   pugi::xml_node findNode;
   pugi::xml_node rNode;

   //Convert wstring to pugi::char_t*
   std::string strXpath = m_Converter.WideStringToString(xpath);
   const pugi::char_t* pugiXpath = strXpath.c_str();   

   if(NULL == m_XmlDocument)
      return -1;

   retVal = 1;

   //check if nodelist is empty then fail  
   xPathNodeSet = m_XmlDocument->select_nodes(pugiXpath);

   int size = xPathNodeSet.size();   
   if(1 > xPathNodeSet.size())
   {
      retVal = -1; goto CleanUp; 
   }

   if (NULL != node.m_XmlNode)
   {
#ifdef DEBUG
      assert(0);
#else
    // Some logging here
#endif
   }

   //Search nodelist for the node with index 
   findNode = m_XmlDocument->first_element_by_path(pugiXpath);

   rNode = findNode.first_child();
   if(NULL == rNode)
   {
       retVal = -1; goto CleanUp;
   }
   findNode = rNode;
   node.m_XmlNode = &rNode;

   for(pugi::xml_node_iterator it = findNode.begin(); it != findNode.end(); ++it)
   {
      nodeIndex++;
      if(nodeIndex == index)
      {
          break;
      }

      //get next sibling of the node. when index matches its corressponding node is already stored
      //so stop searching.
      rNode = findNode.next_sibling();
      if(NULL == rNode)
      {
         retVal = -1; goto CleanUp;
      }
      findNode = rNode;
      node.m_XmlNode = &rNode;

   }

   if(NULL == node.m_XmlNode)
   {
       retVal = -1; goto CleanUp;
   }

CleanUp:
   return retVal;
}

when we are calling GetChildNode like following:

std::wstring xml = 
         L"<?xml version=\"1.0\" encoding=\"utf-8\"?>\
          <testnodes>\
          <nodecategory1>\
          <node attribute1=\"valueA\" attribute2=\"valueB\" attribute3=\"valueC\">nodevalue1</node>\
          </nodecategory1>\
          <nodecategory2>\
          <node attribute1=\"valueD\" attribute2=\"valueE\" attribute3=\"valueF\">nodevalue2</node>\
          <node attribute1=\"valueG\" attribute2=\"valueH\" attribute3=\"valueI\">nodevalue3</node>\
          </nodecategory2>\
          <nodecategory3>\
          <node attribute1=\"valueJ\" attribute2=\"valueK\" attribute3=\"valueL\">nodevalue4</node>\
          </nodecategory3>\
          </testnodes>";

 CXmlDocument testdoc;
 CXmlNode testnode;

(1 == testdoc.GetChildNode(L"/testnodes/nodecategory2/node", 1, testnode) ? 
std::cout << "1": std::cout << "0");

Problem:  When I go past above line the content of testnode get null. The 
content should persist.

What could be the problem?

Original issue reported on code.google.com by rajesh.s...@gmail.com on 17 Sep 2012 at 7:42

[GoogleCodeExporter]

You are returning pointers to local variables from the function:

CXmlDocument::GetChildNode(const std::wstring& xpath,uint index, CXmlNode& node)
{
...
   pugi::xml_node rNode;
   ...
   node.m_XmlNode = &rNode;
...
}

It's not safe. Don't do that.
Also you don't need to use first_element_by_path and select_nodes at the same 
time - just pick one. first_element_by_path does not use XPath, select_nodes 
does.

Original comment by arseny.k...@gmail.com on 17 Sep 2012 at 8:31

• Changed state: Invalid

[GoogleCodeExporter]

I have changed code to following. It returns garbage in node. Please read 
CDS_RETVAL as int and CDS_UINT32 as uint

CDS_RETVAL CXmlDocument::GetChildNode(const std::wstring& xpath, CDS_UINT32 
index, CXmlNode& node) const
{
   CConverter m_Converter;          
   CDS_UINT32 nodeIndex = 0;
   pugi::xpath_node_set xPathNodeSet;
   CDS_RETVAL retVal;
   pugi::xml_node findNode;

   //Convert wstring to pugi::char_t*
   std::string strXpath = m_Converter.WideStringToString(xpath);
   const pugi::char_t* pugiXpath = strXpath.c_str();   

   if(NULL == m_XmlDocument)
      return CDS_ERROR_INSTANCE;

   retVal = OK;

   //check if nodelist is empty then fail  
   xPathNodeSet = m_XmlDocument->select_nodes(pugiXpath);

   int size = xPathNodeSet.size();   
   if(1 > xPathNodeSet.size())
   {
      retVal = CDS_ERROR_VALUE_NOT_FOUND; goto CleanUp; 
   }

   //retVal = GetChildNodes(xpath,nodeList);
   if (NULL != node.m_XmlNode)
   {
#ifdef DEBUG
      assert(0);
#else
      m_log.AddLog(cModuleUtility, cLevelError, "MEMORY LEAK in CXmlDocument::GetChildNode");
#endif
   }

   //Search nodelist for the node with index 
   node.m_XmlNode =  &m_XmlDocument->first_element_by_path(pugiXpath);
   if(NULL == node.m_XmlNode)
   {
         retVal = CDS_ERROR_INSTANCE; goto CleanUp;
   }
   node.m_XmlNode = &xPathNodeSet[index].node() ;//.operator [index-1];
   if(NULL == node.m_XmlNode)
   {
         retVal = CDS_ERROR_INSTANCE; goto CleanUp;
   }CleanUp:
   return retVal;
}

Original comment by rajesh.s...@gmail.com on 17 Sep 2012 at 9:18

[GoogleCodeExporter]

This code has the same issue. xml_node objects are temporary; the real data is 
stored in an internal object, xml_node has a pointer to it.

If you have a pointer to xml_node anywhere in your program except for output 
parameters in a function, it is probably a mistake. Replace all pointers to 
xml_node with values; don't hesitate to copy xml_node objects (it's almost 
free).

Original comment by arseny.k...@gmail.com on 17 Sep 2012 at 9:20

[GoogleCodeExporter]

Will appreciate if you show it by example.

Thanks and Best Regards,
Rajesh

Original comment by rajesh.s...@gmail.com on 18 Sep 2012 at 3:43

[GoogleCodeExporter]

Here's how I would write this code.

struct CXmlNode
{
    pugi::xml_node m_XmlNode;
};

CDS_RETVAL CXmlDocument::GetChildNode(const std::wstring& xpath, CDS_UINT32 
index, CXmlNode& node) const
{
    if (NULL == m_XmlDocument)
        return CDS_ERROR_INSTANCE;

    std::string query = pugi::as_utf8(xpath);
    pugi::xpath_node_set nodes = m_XmlDocument->select_nodes(query.c_str());

    if (index >= nodes.size())
        return CDS_ERROR_VALUE_NOT_FOUND 

    node.m_XmlNode = nodes[index].node();

    if (!node.m_XmlNode)
        return CDS_ERROR_INSTANCE;

    return OK;
}

Original comment by arseny.k...@gmail.com on 18 Sep 2012 at 3:57

[GoogleCodeExporter]

Original comment by arseny.k...@gmail.com on 18 Sep 2012 at 3:58

• Changed state: Done
• Added labels: Type-Task
• Removed labels: Type-Defect