- Fix several bugs causing memory leak or invalid access detected

by Valgrind. This concerns the XML libxml2 support.

modified:
  storage/connect/domdoc.cpp
  storage/connect/domdoc.h
  storage/connect/ha_connect.cc
  storage/connect/libdoc.cpp
  storage/connect/plgdbsem.h
  storage/connect/plgxml.h
  storage/connect/tabxml.cpp
  storage/connect/tabxml.h

storage/connect/domdoc.cpp

@@ -592,6 +592,18 @@ PXNODE DOMNODELIST::GetItem(PGLOBAL g, int n, PXNODE np)
 
   }  // end of GetItem
 
+/******************************************************************/
+/*  Reset the pointer on the deleted item.                        */
+/******************************************************************/
+bool DOMNODELIST::DropItem(PGLOBAL g, int n)
+  {
+  if (Listp == NULL || Listp->length <= n)
+    return true;
+
+//Listp->item[n] = NULL;  La propri<72>t<EFBFBD> n'a pas de m<>thode 'set'
+  return false;
+  }  // end of DeleteItem
+
 /* ----------------------- class DOMATTR ------------------------ */
 
 /******************************************************************/

storage/connect/domdoc.h

@@ -104,8 +104,9 @@ class DOMNODELIST : public XMLNODELIST {
   friend class DOMNODE;
  public:
   // Methods
-  virtual int   GetLength(void) {return Listp->length;}
+  virtual int    GetLength(void) {return Listp->length;}
   virtual PXNODE GetItem(PGLOBAL g, int n, PXNODE np);
+  virtual bool   DropItem(PGLOBAL g, int n);
 
  protected:
   // Constructor

storage/connect/ha_connect.cc

@@ -1093,7 +1093,8 @@ PTDB ha_connect::GetTDB(PGLOBAL g)
 
   if (!xp->CheckQuery(valid_query_id) && tdbp
                       && !stricmp(tdbp->GetName(), table_name)
-                      && tdbp->GetMode() == xmod) {
+                      && (tdbp->GetMode() == xmod 
+                       || tdbp->GetAmType() == TYPE_AM_XML)) {
     tp= tdbp;
     tp->SetMode(xmod);
   } else if ((tp= CntGetTDB(g, table_name, xmod, this)))
@@ -2658,8 +2659,9 @@ int ha_connect::delete_all_rows()
   PGLOBAL g= xp->g;
   DBUG_ENTER("ha_connect::delete_all_rows");
 
-  // Close and reopen the table so it will be deleted
+  if (tdbp && tdbp->GetAmType() != TYPE_AM_XML)
-  rc= CloseTable(g);
+    // Close and reopen the table so it will be deleted
+    rc= CloseTable(g);
 
   if (!(OpenTable(g))) {
     if (CntDeleteRow(g, tdbp, true)) {

storage/connect/libdoc.cpp

@@ -14,6 +14,10 @@
 #include "my_global.h"
 //#endif  // !WIN32
 
+#if !defined(LIBXML_TREE_ENABLED) || !defined(LIBXML_OUTPUT_ENABLED)
+#error "tree support not compiled in"
+#endif
+
 #if !defined(LIBXML_XPATH_ENABLED) || !defined(LIBXML_SAX1_ENABLED)
 #error "XPath not supported"
 #endif
@@ -47,8 +51,6 @@ typedef struct _x2block {         /* Loaded XML file block        */
   short              Type;        /* TYPE_FB_XML                  */
   int                Retcode;     /* Return code from Load        */
   xmlDocPtr          Docp;        /* Document interface pointer   */
-//  xmlXPathContextPtr Ctxp;
-//  xmlXPathObjectPtr  Xop;
   } X2BLOCK, *PX2BLOCK;
 
 /******************************************************************/
@@ -91,6 +93,8 @@ class LIBXMLDOC : public XMLDOCUMENT {
   xmlNodeSetPtr      Nlist;
   xmlXPathContextPtr Ctxp;
   xmlXPathObjectPtr  Xop;
+  xmlXPathObjectPtr  NlXop;
+  xmlErrorPtr        Xerr;
   char              *Buf;                  // Temporary
   bool               Nofreelist;
 }; // end of class LIBXMLDOC
@@ -141,6 +145,7 @@ class XML2NODELIST : public XMLNODELIST {
   // Methods
   virtual int    GetLength(void);
   virtual PXNODE GetItem(PGLOBAL g, int n, PXNODE np);
+  virtual bool   DropItem(PGLOBAL g, int n);
 
  protected:
   // Constructor
@@ -180,6 +185,23 @@ extern int  trace;
 } //   "C"
 
 #if defined(MEMORY_TRACE)
+static int  m = 0;
+static char s[500];
+/**************************************************************************/
+/*  Tracing output function.                                              */
+/**************************************************************************/
+void xtrc(char const *fmt, ...)
+  {
+  va_list ap;
+  va_start (ap, fmt);
+
+//vfprintf(stderr, fmt, ap);
+  vsprintf(s, fmt, ap);
+  if (s[strlen(s)-1] == '\n')
+      s[strlen(s)-1] = 0;
+  va_end (ap);
+  } // end of htrc
+
 static xmlFreeFunc Free; 
 static xmlMallocFunc Malloc;
 static xmlMallocFunc MallocA; 
@@ -188,42 +210,53 @@ static xmlStrdupFunc Strdup;
 
 void xmlMyFree(void *mem)
 {
-  if (trace) 
+  if (trace) { 
-    htrc("Freeing          at %p\n", mem);
+    htrc("%.4d Freeing          at %p   %s\n", ++m, mem, s);
+    *s = 0;
+    } // endif trace
   Free(mem);
 } // end of xmlMyFree
 
 void *xmlMyMalloc(size_t size)
 {
   void *p = Malloc(size);
-  if (trace) 
+  if (trace) { 
-    htrc("Allocating %.5d at %p\n", size, p);
+    htrc("%.4d Allocating %.5d at %p   %s\n", ++m, size, p, s);
+    *s = 0;
+    } // endif trace
   return p;
 } // end of xmlMyMalloc
 
 void *xmlMyMallocAtomic(size_t size)
 {
   void *p = MallocA(size);
-  if (trace) 
+  if (trace) { 
-    htrc("Atom alloc %.5d at %p\n", size, p);
+    htrc("%.4d Atom alloc %.5d at %p   %s\n", ++m, size, p, s);
+    *s = 0;
+    } // endif trace
   return p;
 } // end of xmlMyMallocAtomic
 
 void *xmlMyRealloc(void *mem, size_t size)
 {
   void *p = Realloc(mem, size);
-  if (trace) 
+  if (trace) { 
-    htrc("ReAlloc    %.5d to %p from %p\n", size, p, mem);
+    htrc("%.4d ReAlloc    %.5d to %p from %p   %s\n", ++m, size, p, mem, s);
+    *s = 0;
+    } // endif trace
   return p;
 } // end of xmlMyRealloc
 
 char *xmlMyStrdup(const char *str)
 {
   char *p = Strdup(str);
-  if (trace) 
+  if (trace) { 
-    htrc("Duplicating      to %p from %p %s\n", p, str, str);
+    htrc("%.4d Duplicating      to %p from %p %s   %s\n", ++m, p, str, str, s);
+    *s = 0;
+    } // endif trace
   return p;
 } // end of xmlMyStrdup
+#define htrc xtrc
 #endif   // MEMORY_TRACE
 
 /******************************************************************/
@@ -295,6 +328,8 @@ LIBXMLDOC::LIBXMLDOC(char *nsl, char *nsdf, char *enc, PFBLOCK fp)
   Nlist = NULL;
   Ctxp = NULL;
   Xop = NULL;
+  NlXop = NULL;
+  Xerr = NULL;
   Buf = NULL;
   Nofreelist = false;
   } // end of LIBXMLDOC constructor
@@ -321,9 +356,10 @@ bool LIBXMLDOC::ParseFile(char *fn)
       Encoding = (char*)Docp->encoding;
 
     return false;
-  } else
+  } else if ((Xerr = xmlGetLastError()))
-    return true;
+    xmlResetError(Xerr);
 
+  return true;
   } // end of ParseFile
 
 /******************************************************************/
@@ -344,8 +380,6 @@ PFBLOCK LIBXMLDOC::LinkXblock(PGLOBAL g, MODE m, int rc, char *fn)
   xp->Length = (m == MODE_READ) ? 1 : 0;
   xp->Retcode = rc;
   xp->Docp = Docp;
-//  xp->Ctxp = Ctxp;
-//  xp->Xop = Xop;
 
   // Return xp as a fp
   return (PFBLOCK)xp;
@@ -356,6 +390,9 @@ PFBLOCK LIBXMLDOC::LinkXblock(PGLOBAL g, MODE m, int rc, char *fn)
 /******************************************************************/
 bool LIBXMLDOC::NewDoc(PGLOBAL g, char *ver)
   {
+  if (trace)
+    htrc("NewDoc\n");
+
   return ((Docp = xmlNewDoc(BAD_CAST ver)) == NULL);
   } // end of NewDoc
 
@@ -462,8 +499,7 @@ int LIBXMLDOC::DumpDoc(PGLOBAL g, char *ofn)
   if (xmlSaveFormatFileEnc((const char *)ofn, Docp, Encoding, 0) < 0) {
     xmlErrorPtr err = xmlGetLastError();
 
-    strcpy(g->Message, (err) ? err->message : "Error saving XML doc"
+    strcpy(g->Message, (err) ? err->message : "Error saving XML doc");
-      );
     rc = -1;
     } // endif Save
 //  rc = xmlDocDump(of, Docp);
@@ -497,17 +533,40 @@ void LIBXMLDOC::CloseDoc(PGLOBAL g, PFBLOCK xp)
   if (trace)
     htrc("CloseDoc: xp=%p count=%d\n", xp, (xp) ? xp->Count : 0);
 
-  if (xp && xp->Count == 1) {
+//if (xp && xp->Count == 1) {
-    if (Nlist)
+    if (Nlist) {
       xmlXPathFreeNodeSet(Nlist);
 
-    if (Xop)
+      if ((Xerr = xmlGetLastError()))
+        xmlResetError(Xerr);
+
+      } // endif Nlist
+
+    if (Xop) {
       xmlXPathFreeObject(Xop);
 
-    if (Ctxp)
+      if ((Xerr = xmlGetLastError()))
+        xmlResetError(Xerr);
+
+      } // endif Xop
+
+    if (NlXop) {
+      xmlXPathFreeObject(NlXop);
+
+      if ((Xerr = xmlGetLastError()))
+        xmlResetError(Xerr);
+
+      } // endif NlXop
+
+    if (Ctxp) {
       xmlXPathFreeContext(Ctxp);
 
-    } // endif Count
+      if ((Xerr = xmlGetLastError()))
+        xmlResetError(Xerr);
+
+      } // endif Ctxp
+
+//  } // endif Count
 
   CloseXML2File(g, xp, false);
   } // end of Close
@@ -560,18 +619,29 @@ xmlNodeSetPtr LIBXMLDOC::GetNodeList(PGLOBAL g, xmlNodePtr np, char *xp)
 
       } // endfor nsp
 
-  } else {
+    } // endif Ctxp
+
+  if (Xop) {
     if (trace)
-      htrc("Calling xmlXPathFreeNodeSetList Xop=%p\n", Xop);
+      htrc("Calling xmlXPathFreeNodeSetList Xop=%p NOFREE=%d\n", 
+                                            Xop, Nofreelist);
 
     if (Nofreelist) {
       // Making Nlist that must not be freed yet
-      xmlXPathFreeNodeSetList(Xop);       // Caused memory leak
+//    xmlXPathFreeNodeSetList(Xop);       // Caused memory leak
+      assert(!NlXop);
+      NlXop = Xop;                        // Freed on closing
       Nofreelist = false;
     } else
       xmlXPathFreeObject(Xop);            // Caused node not found
 
-  } // endif Ctxp
+    if ((Xerr = xmlGetLastError())) {
+      strcpy(g->Message, Xerr->message);
+      xmlResetError(Xerr);
+      return NULL;
+      } // endif Xerr
+
+    } // endif Xop
 
   // Set the context to the calling node
   Ctxp->node = np;
@@ -990,6 +1060,8 @@ void XML2NODE::AddText(PGLOBAL g, char *txtp)
 /******************************************************************/
 void XML2NODE::DeleteChild(PGLOBAL g, PXNODE dnp)
   {
+  xmlErrorPtr xerr;
+
   if (trace)
     htrc("DeleteChild: node=%p\n", dnp);
 
@@ -999,12 +1071,39 @@ void XML2NODE::DeleteChild(PGLOBAL g, PXNODE dnp)
   // This is specific to row nodes
   if (text && text->type == XML_TEXT_NODE) {
     xmlUnlinkNode(text);
+
+    if ((xerr = xmlGetLastError()))
+      goto err;
+
     xmlFreeNode(text);
+
+    if ((xerr = xmlGetLastError()))
+      goto err;
+
     } // endif type
 
   xmlUnlinkNode(np);
+
+  if ((xerr = xmlGetLastError()))
+    goto err;
+
   xmlFreeNode(np);
+
+  if ((xerr = xmlGetLastError()))
+    goto err;
+
   Delete(dnp);
+
+  if ((xerr = xmlGetLastError()))
+    goto err;
+
+  return;
+
+err:
+  if (trace)
+    htrc("DeleteChild: errmsg=%s\n", xerr->message);
+
+  xmlResetError(xerr);
   } // end of DeleteChild
 
 /* -------------------- class XML2NODELIST ---------------------- */
@@ -1045,6 +1144,22 @@ PXNODE XML2NODELIST::GetItem(PGLOBAL g, int n, PXNODE np)
 
   } // end of GetItem
 
+/******************************************************************/
+/*  Reset the pointer on the deleted item.                        */
+/******************************************************************/
+bool XML2NODELIST::DropItem(PGLOBAL g, int n)
+  {
+  if (trace)
+    htrc("DropItem: n=%d\n", n);
+
+  // We should do something here
+  if (!Listp || Listp->nodeNr <= n)
+    return true;
+
+  Listp->nodeTab[n] = NULL;   // This was causing Valgrind warning
+  return false;
+  } // end of DropItem
+
 /* ---------------------- class XML2ATTR ------------------------ */
 
 /******************************************************************/

storage/connect/plgdbsem.h

@@ -111,6 +111,7 @@ enum AMT {TYPE_AM_ERROR =   0,        /* Type not defined              */
           TYPE_AM_PIVOT = 120,        /* PIVOT access method type no   */
           TYPE_AM_SRC   = 121,        /* PIVOT multiple column type no */
           TYPE_AM_FNC   = 122,        /* PIVOT source column type no   */
+          TYPE_AM_XML   = 127,        /* XML access method type no     */
           TYPE_AM_XTB   = 130,        /* SYS table access method type  */
           TYPE_AM_MAC   = 137,        /* MAC table access method type  */
           TYPE_AM_WMI   = 139,        /* WMI table access method type  */

storage/connect/plgxml.h

@@ -147,6 +147,7 @@ class XMLNODELIST : public BLOCK {
   // Properties
   virtual int    GetLength(void) = 0;
   virtual PXNODE GetItem(PGLOBAL, int, PXNODE = NULL) = 0;
+  virtual bool   DropItem(PGLOBAL, int) = 0;
 
  protected:
   // Constructor

storage/connect/tabxml.cpp

@@ -78,7 +78,9 @@ XMLDEF::XMLDEF(void)
   DefNs = NULL;
   Attrib = NULL;
   Hdattr = NULL;
+  Coltype = 1;
   Limit = 0;
+  Header = 0;
   Xpand = false;
   Usedom = false;
   } // end of XMLDEF constructor
@@ -338,17 +340,14 @@ PCOL TDBXML::InsertSpecialColumn(PGLOBAL g, PCOL colp)
 /***********************************************************************/
 /*  LoadTableFile: Load and parse an XML file.                         */
 /***********************************************************************/
-int TDBXML::LoadTableFile(PGLOBAL g)
+int TDBXML::LoadTableFile(PGLOBAL g, char *filename)
   {
-  char    filename[_MAX_PATH];
   int     rc = RC_OK, type = (Usedom) ? TYPE_FB_XML : TYPE_FB_XML2;
   PFBLOCK fp = NULL;
   PDBUSER dup = (PDBUSER)g->Activityp->Aptr;
 
-  /*********************************************************************/
+  if (Docp)
-  /*  We used the file name relative to recorded datapath.             */
+    return rc;               // Already done
-  /*********************************************************************/
-  PlugSetPath(filename, Xfile, GetPath());
 
   if (trace)
     htrc("TDBXML: loading %s\n", filename);
@@ -397,6 +396,7 @@ int TDBXML::LoadTableFile(PGLOBAL g)
       } else
         rc = (errno == ENOENT) ? RC_NF : RC_INFO;
 
+      // Cannot make a Xblock until document is made
       return rc;
       } // endif Docp
 
@@ -418,9 +418,8 @@ int TDBXML::LoadTableFile(PGLOBAL g)
 /***********************************************************************/
 bool TDBXML::Initialize(PGLOBAL g)
   {
-  char       tabpath[64];
+  int     rc;
-  int        rc;
+  PXMLCOL colp;
-  PXMLCOL   colp;
 
   if (Void)
     return false;
@@ -440,8 +439,13 @@ bool TDBXML::Initialize(PGLOBAL g)
 #else
   if (!Root) {
 #endif
+    char tabpath[64], filename[_MAX_PATH];
+
+    //  We used the file name relative to recorded datapath
+    PlugSetPath(filename, Xfile, GetPath());
+
     // Load or re-use the table file
-    rc = LoadTableFile(g);
+    rc = LoadTableFile(g, filename);
 
     if (rc == RC_OK) {
       // Get root node
@@ -503,6 +507,9 @@ bool TDBXML::Initialize(PGLOBAL g)
           goto error;
           } // endif NewDoc
 
+        //  Now we can link the Xblock
+        To_Xb = Docp->LinkXblock(g, Mode, rc, filename);
+
         // Add a CONNECT comment node
 //      sprintf(buf, MSG(CREATED_PLUGDB), version);
         sprintf(buf, " Created by CONNECT %s ", version);
@@ -893,12 +900,21 @@ int TDBXML::DeleteDB(PGLOBAL g, int irc)
       if ((RowNode = Nlist->GetItem(g, Irow, RowNode)) == NULL) {
         sprintf(g->Message, MSG(MISSING_ROWNODE), Irow);
         return RC_FX;
-      } else
+      } else {
         TabNode->DeleteChild(g, RowNode);
 
+        if (Nlist->DropItem(g, Irow))
+          return RC_FX;
+
+      } // endif RowNode
+
     Changed = true;
   } else if (irc != RC_EF) {
     TabNode->DeleteChild(g, RowNode);
+
+    if (Nlist->DropItem(g, Irow))
+      return RC_FX;
+
     Changed = true;
   } // endif's irc
 

storage/connect/tabxml.h

@@ -6,8 +6,6 @@
 /*                                                                     */
 /*  This file contains the XML table classes declares.                 */
 /***********************************************************************/
-#define TYPE_AM_XML  (AMT)127
-
 typedef class XMLDEF *PXMLDEF;
 typedef class TDBXML *PTDBXML;
 typedef class XMLCOL *PXMLCOL;
@@ -81,7 +79,7 @@ class DllExport TDBXML : public TDBASE {
   virtual void  ResetDB(void) {N = 0;}
   virtual void  ResetSize(void) {MaxSize = -1;}
   virtual int   RowNumber(PGLOBAL g, bool b = false);
-          int   LoadTableFile(PGLOBAL g);
+          int   LoadTableFile(PGLOBAL g, char *filename);
           bool  Initialize(PGLOBAL g);
           bool  SetTabNode(PGLOBAL g);
           void  SetNodeAttr(PGLOBAL g, char *attr, PXNODE node);