Re: Bugs in HTAnchor_delete

Hi Á¤½Âȯ, hi all.

I've mentioned the first bug. Thanks.
I agree with the second bug you've found. Congratulation.

Well. It seems that we are two and that we agree on a bug fix.

"Again, I tell you that if two of you agree about anything you ask for, 
 it will be done for you [...]" (Mat 18:19)

----

I'm not sure that fixing these two bugs is enough 
to make HTAnchor_delete a safe procedure.

What, in the HTAnchor_delete algorithm, makes that 
the 'sources' list will be empty?
This is not the case when one did multiple GET requests.

Let have an page with an anchor to itself. 
(like the second anchor of the late "copyright" paragraph of the
www.w3.org page)
If you have got any other page with an anchor to the www.w3.org page 
and you try to delete this anchor, what will happen?

As the algorithm is recursive and one are not sure that all the anchors
could be deleted it would be a good idea to set to NULL the lists which 
had been deleted.


> Á¤½Âȯ wrote:
> 
> There seem to be two bugs in HTAnchor_delete in HTAnchor.c. I think
> that one of the bugs has been already mentioned by someone on this
> mailing list.
> 
> On lines 503 - 506,
> 
>             while ((child=(HTChildAnchor *)
> HTList_removeLastObject(kids)))
>             delete_links((HTAnchor *) child);
>             HT_FREE(child->tag);
>             HT_FREE(child);
> 
> I believe this is meant to be
> 
>             while ((child=(HTChildAnchor *)
> HTList_removeLastObject(kids))) {
>             delete_links((HTAnchor *) child);
>             HT_FREE(child->tag);
>             HT_FREE(child);
>             }
> 
> The other is a wrong placement of a bracket.
> 
>     if (!HTList_isEmpty(me->sources)) {    /* There are still incoming
> links */
> 
>     /*
>     ** Delete all outgoing links from children, if any
>     */
>     if (me->children) {
>         int cnt = 0;
>         for (; cnt<CHILD_HASH_SIZE; cnt++) {
>         HTList * kids = me->children[cnt];
>         if (kids) {
>             HTChildAnchor * child;
>             while ((child = (HTChildAnchor *)
> HTList_nextObject(kids)))
>             delete_links((HTAnchor *) child);
>             return NO;  /* Parent not deleted */
>         }
>         }
>     }
> 
>     /*
>     ** No more incoming links : kill everything
>     ** First, recursively delete children
>     */
>     if (me->children) {
>         int cnt = 0;
>         for (; cnt<CHILD_HASH_SIZE; cnt++) {
>         HTList * kids = me->children[cnt];
>         if (kids) {
>             HTChildAnchor * child;
>             while ((child=(HTChildAnchor *)
> HTList_removeLastObject(kids)))
>             delete_links((HTAnchor *) child);
>             HT_FREE(child->tag);
>             HT_FREE(child);
>         }
>         }
>     }
>     } <----
> 
> The comments indicate that the anchor is allowed to be erased only
> when there are no more incoming sources for the anchor. However, "if
> (!HTList_isEmpty(me->sources)) {" encloses statements that belong to
> the no-more-incoming-links case. The bracket at the line 510 should
> probably be moved somewhere around the line 492.

Received on Sunday, 21 October 2001 19:22:17 UTC