- From: Vic Bancroft <bancroft@america.net>
- Date: Sun, 09 Jul 2006 20:58:04 -0400
- To: Sam Varshavchik <mrsam@courier-mta.com>
- CC: www-lib@w3.org, Arthur Smith <apsmith@aps.org>
Sam Varshavchik wrote:
> valgrind comes back with a memory leak in the new version of
> HTBound.c. When I rewrote HTBound.c last year, I copied some snippets
> from the old version into the new version on HTBound.c, I guess
> without fully understanding other parts of libwww.
No doubt, there is a lot to read in Library/src !
> Can't really say for sure, but the old HTBound.c might've also been
> leaking in the same spot. The analogous HTStreamStack call in the old
> HTBound.c had the same parameters.
An HTStreamStack is constructed for format conversions, but most callers
do not use the fancy HTMerge action like your invocation. The only
other usage of HTMerge is an interesting section in HTMIME when there is
the explict handling of two feeds, one from the network and one from the
cache. It would appear that in the context of HTBound there is only one
feed. Egads, how did we not see this before :^?
> Anyway, this patch makes valgrind happy. Perhaps someone who's more
> familiar with HTMerge can share a comment, here.
Most excellent ! It is now in the HEAD as new revision: 2.16. There
were also some "#if 0" snippets in HTMIME.c which could use some
trimming, giving us a new revision: 2.102 . . .
Taken together cvs diff looked like,
Index: Library/src/HTBound.c
===================================================================
RCS file: /sources/public/libwww/Library/src/HTBound.c,v
retrieving revision 2.15
diff -r2.15 HTBound.c
412c412
< HTMerge(me->orig_target, 2),
---
> HTMerge(me->orig_target, 1),
[bancroft@hilbert libwww]$ cvs diff lLibrary/src/HTMIME.c
cvs [diff aborted]: no such directory `lLibrary/src'
[bancroft@hilbert libwww]$ cvs diff Library/src/HTMIME.c
Index: Library/src/HTMIME.c
===================================================================
RCS file: /sources/public/libwww/Library/src/HTMIME.c,v
retrieving revision 2.101
diff -r2.101 HTMIME.c
198,201d197
< #if 0
< /* @@ JK: change */
< if (append) me->target = append;
< #endif
661c657
< HTTRACE(STREAM_TRACE, "Cache Flush. Flushing and freeing
PIPE buffer\n");
---
> HTTRACE(STREAM_TRACE, "Cache Flush. Flushing PIPE buffer\n");
663,667d658
< #if 0
< /* @@ JK: flush converts the pipe to an open one, we shouldn't
< free it as we'll loose our references */
< (*pipe->isa->_free)(pipe);
< #endif
720,731d710
< #if 0
< /* JK: this doesn't work because this work is repeated before */
< /*
< ** Create the cache append stream, and a Tee stream
< */
< {
< HTStream * append = HTStreamStack(WWW_CACHE_APPEND,
output_format,
< output_stream, request, NO);
< if (append) me->target = HTTee(me->target, append, NULL);
< }
< #endif
<
more,
l8r,
v
BTW, is it time to revisit the release party idea as per ChangeLog
revision 1.57 should we continue to worry over modification to address
Ben's security concerns? Along those lines, did anyone come up with an
appropriate Solaris box to construct a patch or vet a clean build as per
the errors experienced by Arthur Smith ?
--
"The future is here. It's just not evenly distributed yet."
-- William Gibson, quoted by Whitfield Diffie
Received on Monday, 10 July 2006 00:58:18 UTC