Re: [xslt] str:replace implementation



Daniel Veillard wrote:
On Mon, Jan 08, 2007 at 04:04:40PM -0500, joel reed wrote:
Please find attached an implementation of the exslt.org str:replace function and 7 test cases.

  Okay, thanks !

Documentation on the function can be found here:
  http://www.exslt.org/str/functions/replace/index.html

In that spec, I did not implement these last two sentences: "The longest search strings are replaced first. If a search string is empty, then the equivalently positioned replacement node is inserted between every character in the string." I felt the added code complexity and performance hit was not worth the impact at this time.

  I guess there is a bug there:
    xmlUTF8Strlen() counts the number of characters in an UTF-8 encoded
    string, not the number of bytes. So you should not use it as the
    value of the increment of the xmlChar * , but use xmlStrlen() instead in
    exsltStrReplaceInternal()

fixed in attached patch.

I also added the rest to the makefile.am in tests/exslt/strings, in my version
compiled with memory debug it points out a memory leak:

fixed in attached patch.


paphio:~/XSLT/tests/exslt/strings -> make tests
## Running exslt string tests
replace.1 result
MEMORY ALLOCATED : 1218, MAX was 171024
paphio:~/XSLT/tests/exslt/strings -> cat .memdump 03:04:43 PM

      MEMORY ALLOCATED : 1218, MAX was 171020
BLOCK  NUMBER   SIZE  TYPE
0        1203     11 malloc()  in none(0) "j, j, j, j"
1        1200     66 malloc()  in none(0) "fum"
2        1197     66 malloc()  in none(0) "fo"
3        1194     66 malloc()  in none(0) "fi"
4        1191     66 malloc()  in none(0) "fee"
5        1188     17 malloc()  in none(0) "fee, fi, fo, fum"
6        1187      2 malloc()  in none(0) "j"
7        1185     40 malloc()  in none(0)
8        1184     12 malloc()  in none(0) pointer to #1185 at index 8
9        1179     34 malloc()  in none(0) "tee, eye, billow, a longe"
10       1176     66 malloc()  in none(0) "a longer string"
11       1174     66 malloc()  in none(0) "fum"
12       1171     66 malloc()  in none(0) "billow"
13       1169     66 malloc()  in none(0) "fo"
etc...

  I start looking at the code with the debugger (following the instructions
at http://xmlsoft.org/xmlmem.html) and it is clear:
   - that the argument popped from XPath stack are not freed in the normal path
   - it seems that there is no free either in case of error, just doing
     xmlXPathSetTypeError() and return but no deallocations

Plus there is a number of case, where the args can be strings or nodesets, it's
probably better if you could check all the deallocation paths and provide
another patch once those issues have been checked that would be nice !

fixed in attached patch.

thanks again for the feedback. when i run make tests now i get an empty .memdump file. i hope this version looks better to you.

jr
diff --git a/libexslt/strings.c b/libexslt/strings.c
index c867f9c..af7aa88 100644
--- a/libexslt/strings.c
+++ b/libexslt/strings.c
@@ -495,6 +495,143 @@ exsltStrConcatFunction (xmlXPathParserCo
 }
 
 /**
+ * exsltStrReplaceInternal:
+ * @str: string to modify
+ * @searchStr: string to find
+ * @replaceStr: string to replace occurrences of searchStr
+ *
+ * Search and replace string function used by exsltStrReplaceFunction
+ */
+static xmlChar*
+exsltStrReplaceInternal(const xmlChar* str, const xmlChar* searchStr, 
+                        const xmlChar* replaceStr)
+{
+    const xmlChar *curr, *next;
+    xmlChar *ret = NULL;
+    int searchStrSize;
+
+    curr = str;
+    searchStrSize = xmlStrlen(searchStr);
+
+    do {
+      next = xmlStrstr(curr, searchStr);
+      if (next == NULL) {
+        ret = xmlStrcat (ret, curr);
+        break;
+      }
+
+      ret = xmlStrncat (ret, curr, next - curr);
+      ret = xmlStrcat (ret, replaceStr);
+      curr = next + searchStrSize;
+    } while (*curr != 0);
+
+    return ret;
+}
+/**
+ * exsltStrReplaceFunction:
+ * @ctxt: an XPath parser context
+ * @nargs: the number of arguments
+ *
+ * Takes a string, and two node sets and returns the string with all strings in 
+ * the first node set replaced by all strings in the second node set.
+ */
+static void
+exsltStrReplaceFunction (xmlXPathParserContextPtr ctxt, int nargs) {
+    xmlChar *str = NULL, *searchStr = NULL, *replaceStr = NULL;
+    xmlNodeSetPtr replaceSet = NULL, searchSet = NULL;
+    xmlChar *ret = NULL, *retSwap = NULL;
+    int i;
+
+    if (nargs  != 3) {
+      xmlXPathSetArityError(ctxt);
+      return;
+    }
+
+    /* pull out replace argument */
+    if (!xmlXPathStackIsNodeSet(ctxt)) {
+      replaceStr = xmlXPathPopString(ctxt);
+    }
+		else {
+      replaceSet = xmlXPathPopNodeSet(ctxt);
+      if (xmlXPathCheckError(ctxt)) {
+        xmlXPathSetTypeError(ctxt);
+        goto fail;
+      }
+    }
+
+    /* behavior driven by search argument from here on */
+    if (!xmlXPathStackIsNodeSet(ctxt)) {
+      searchStr = xmlXPathPopString(ctxt);
+      str = xmlXPathPopString(ctxt);
+
+      if (replaceStr == NULL) {
+        xmlXPathSetTypeError(ctxt);
+        goto fail;
+      }
+
+      ret = exsltStrReplaceInternal(str, searchStr, replaceStr);
+    }
+		else {
+      searchSet = xmlXPathPopNodeSet(ctxt);
+      if (searchSet == NULL || xmlXPathCheckError(ctxt)) {
+        xmlXPathSetTypeError(ctxt);
+        goto fail;
+      }
+
+      str = xmlXPathPopString(ctxt);
+			ret = xmlStrdup(str);
+
+			for (i = 0; i < searchSet->nodeNr; i++) {
+
+				searchStr = xmlXPathCastNodeToString(searchSet->nodeTab[i]);
+
+        if (replaceSet != NULL) {
+          replaceStr = NULL;
+          if (i <= replaceSet->nodeNr) {
+            replaceStr = xmlXPathCastNodeToString(replaceSet->nodeTab[i]);
+          }
+
+          retSwap = exsltStrReplaceInternal(ret, searchStr, replaceStr);
+          
+          if (replaceStr != NULL) {
+            xmlFree(replaceStr);
+            replaceStr = NULL;
+          }
+        }
+        else {
+          retSwap = exsltStrReplaceInternal(ret, searchStr, replaceStr);
+        }
+
+				xmlFree(ret);
+        if (searchStr != NULL) {
+          xmlFree(searchStr);
+          searchStr = NULL;
+        }
+
+				ret = retSwap;
+			}
+
+      if (replaceSet != NULL)
+        xmlXPathFreeNodeSet(replaceSet);
+
+      if (searchSet != NULL)
+        xmlXPathFreeNodeSet(searchSet);
+		}
+
+    xmlXPathReturnString(ctxt, ret);
+
+ fail:
+    if (replaceStr != NULL)
+      xmlFree(replaceStr);
+
+    if (searchStr != NULL)
+      xmlFree(searchStr);
+
+    if (str != NULL)
+      xmlFree(str);
+}
+
+/**
  * exsltStrRegister:
  *
  * Registers the EXSLT - Strings module
@@ -523,4 +660,7 @@ exsltStrRegister (void) {
     xsltRegisterExtModuleFunction ((const xmlChar *) "concat",
 				   EXSLT_STRINGS_NAMESPACE,
 				   exsltStrConcatFunction);
+    xsltRegisterExtModuleFunction ((const xmlChar *) "replace",
+				   EXSLT_STRINGS_NAMESPACE,
+				   exsltStrReplaceFunction);
 }
diff --git a/tests/exslt/strings/Makefile.am b/tests/exslt/strings/Makefile.am
index 6926802..5bdbd2a 100644
--- a/tests/exslt/strings/Makefile.am
+++ b/tests/exslt/strings/Makefile.am
@@ -7,7 +7,8 @@ EXTRA_DIST = 						\
 	tokenize.1.xml tokenize.1.xsl tokenize.1.out	\
 	tokenize.2.xml tokenize.2.xsl tokenize.2.out	\
 	tokenize.3.xml tokenize.3.xsl tokenize.3.out	\
-	split.1.xml split.1.xsl split.1.out
+	split.1.xml split.1.xsl split.1.out \
+	replace.1.xml replace.1.xsl replace.1.out 
 
 all:
 
diff --git a/tests/exslt/strings/Makefile.in b/tests/exslt/strings/Makefile.in
index 167160e..3a112c9 100644
--- a/tests/exslt/strings/Makefile.in
+++ b/tests/exslt/strings/Makefile.in
@@ -213,7 +213,8 @@ EXTRA_DIST = \
 	tokenize.1.xml tokenize.1.xsl tokenize.1.out	\
 	tokenize.2.xml tokenize.2.xsl tokenize.2.out	\
 	tokenize.3.xml tokenize.3.xsl tokenize.3.out	\
-	split.1.xml split.1.xsl split.1.out
+	split.1.xml split.1.xsl split.1.out \
+	replace.1.xml replace.1.xsl replace.1.out 
 
 all: all-am
 
diff --git a/tests/exslt/strings/replace.1.out b/tests/exslt/strings/replace.1.out
new file mode 100644
index 0000000..076d110
--- /dev/null
+++ b/tests/exslt/strings/replace.1.out
@@ -0,0 +1,22 @@
+<?xml version="1.0"?>
+<out>;
+	str:replace('a, simple, list', ', ', '-')
+	a-simple-list
+
+	str:replace('a, simple, list', 'a, ', 'the ')
+	the simple, list
+
+	str:replace('a, simple, list', 'list', 'array')
+	a, simple, array
+
+	str:replace('a, simple, list', 'i', 'I')
+	a, sImple, lIst
+
+	str:replace('a, simple, list', ', ', '')
+	asimplelist
+
+	str:replace('fee, fi, fo, fum', $x, $y)
+	tee, eye, billow, a longer string
+
+	str:replace('fee, fi, fo, fum', $x, 'j')
+	j, j, j, j</out>
diff --git a/tests/exslt/strings/replace.1.xml b/tests/exslt/strings/replace.1.xml
new file mode 100644
index 0000000..6371161
--- /dev/null
+++ b/tests/exslt/strings/replace.1.xml
@@ -0,0 +1,12 @@
+<doc>
+	<strings>
+		<x>fee</x>
+		<x>fi</x>
+		<x>fo</x>
+		<x>fum</x>
+		<y>tee</y>
+		<y>eye</y>
+		<y>billow</y>
+		<y>a longer string</y>
+	</strings>
+</doc>
diff --git a/tests/exslt/strings/replace.1.xsl b/tests/exslt/strings/replace.1.xsl
new file mode 100644
index 0000000..0c9e86c
--- /dev/null
+++ b/tests/exslt/strings/replace.1.xsl
@@ -0,0 +1,36 @@
+<?xml version="1.0"?>
+<xsl:stylesheet version="1.0"
+    xmlns:xsl="http://www.w3.org/1999/XSL/Transform";
+    xmlns:str="http://exslt.org/strings";
+    exclude-result-prefixes="str">
+
+<xsl:template match="/">
+	<xsl:variable name="x" select="doc/strings/x"/>
+	<xsl:variable name="y" select="doc/strings/y"/>
+
+<out>;
+	str:replace('a, simple, list', ', ', '-')
+	<xsl:copy-of select="str:replace('a, simple, list', ', ', '-')"/>
+
+	str:replace('a, simple, list', 'a, ', 'the ')
+	<xsl:copy-of select="str:replace('a, simple, list', 'a, ', 'the ')"/>
+
+	str:replace('a, simple, list', 'list', 'array')
+	<xsl:copy-of select="str:replace('a, simple, list', 'list', 'array')"/>
+
+	str:replace('a, simple, list', 'i', 'I')
+	<xsl:copy-of select="str:replace('a, simple, list', 'i', 'I')"/>
+
+	str:replace('a, simple, list', ', ', '')
+	<xsl:copy-of select="str:replace('a, simple, list', ', ', '')"/>
+
+	str:replace('fee, fi, fo, fum', $x, $y)
+	<xsl:copy-of select="str:replace('fee, fi, fo, fum', $x, $y)" />
+
+	str:replace('fee, fi, fo, fum', $x, 'j')
+	<xsl:copy-of select="str:replace('fee, fi, fo, fum', $x, 'j')" />
+
+</out>
+</xsl:template>
+
+</xsl:stylesheet>
begin:vcard
fn:Joel Reed
n:Reed;Joel
org:Development Dimensions International;E-Systems
adr;dom:;;1225 Washington Pike;Bridgeville;PA;15017
title:Manager, E-Products R&D
tel;work:412-257-3881
note;quoted-printable:Unleashing Executive Talent.  Hiring & Promoting The Best.=0D=0A=
	Developing Extraordinary Leaders.  Discover how at www.ddiworld.com
x-mozilla-html:FALSE
version:2.1
end:vcard



[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]