Re: [xslt] str:replace implementation

["William M. Brack" <wbrack mmm com hk>]

Daniel Veillard wrote:
> On Tue, Jan 09, 2007 at 03:05:57PM -0500, joel reed wrote:
>> 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.
>
>   Way better .... but I still get a memleak, and I don't understand where
> it
> comes from:
>
> paphio:~/XSLT/tests/exslt/strings -> cat .memdump
>       10:47:17 PM
>
> MEMORY ALLOCATED : 121, MAX was 170042
> BLOCK  NUMBER   SIZE  TYPE
> 0        1203     11 malloc()  in none(0) "j, j, j, j"
> 1        1179     34 malloc()  in none(0) "tee, eye, billow, a longe"
> 2        1144     12 malloc()  in none(0) "asimplelist"
> 3        1133     16 malloc()  in none(0) "a, sImple, lIst"
> 4        1122     17 malloc()  in none(0) "a, simple, array"
> 5        1111     17 malloc()  in none(0) "the simple, list"
> 6        1099     14 malloc()  in none(0) "a-simple-list"
> paphio:~/XSLT/tests/exslt/strings ->
>
>   It's actually copies of the string returned by the function, not the
> string
> value itself:
>
> (gdb) where
> #0  xmlMallocBreakpoint () at xmlmemory.c:145
> #1  0x080a1a17 in xmlMallocLoc (size=14, file=0x8139dc1 "none", line=0)
>     at xmlmemory.c:202
> #2  0x080a1a7f in xmlMemMalloc (size=14) at xmlmemory.c:296
> #3  0x080df7c7 in xmlStrndup (cur=0x9eff200 "a-simple-list", len=13)
>     at xmlstring.c:45
> #4  0x080df952 in xmlStrdup (cur=0x1 <Address 0x1 out of bounds>)
>     at xmlstring.c:71
> #5  0x080c3f7d in xmlXPathCastToString (val=0x1) at xpath.c:5621
> #6  0x0806f102 in xsltCopyOf (ctxt=0x9efb430, node=0x9efb598,
> inst=0x9f01630,
>     castedComp=0x9ef84a8) at transform.c:4295
> #7  0x0806bf8f in xsltApplySequenceConstructor (ctxt=0x9efb430,
>     contextNode=0x9efb598, list=0x9f01010, templ=0x9ef78f8) at
> transform.c:2578
> #8  0x0806d1cc in xsltApplyXSLTTemplate (ctxt=0x9efb430,
>     contextNode=0x9efb598, list=0x9f01010, templ=0x9ef78f8,
> withParams=0x0)
>     at transform.c:3026
> #9  0x0806de0c in xsltProcessOneNode (ctxt=0x9efb430,
> contextNode=0x9efb598,
>     withParams=0x0) at transform.c:2030
>
>   the frame #6 is the xsl:copy implementation and that's the one doing
> the
> copy of the value returned on the XPath evaluation. I must be tired I
> can't
> see what's wrong, this has to be related to the way the function work but
> it looks fine and we get the problem when we use the result ... I'm
> puzzled.
>
> Daniel

Looks to me as though the leak has nothing to do with Joel, but rather
comes from transform.c (somewhere around 4307 in SVN).  I'll have that
fixed fairly soon, and get it (together with Joel's patches) committed
later today.

Bill