Re: rcorder(8) dumps core

On Tue, Sep 14, 2004 at 05:52:12PM +0200, Lukas Ertl wrote:
> today I noticed a problem with rcorder on a very recent -CURRENT:
> 
> [root_at_korben ~]# rcorder -k FreeBSD -s nostart -d /etc/rc.d/*
[snip]
> rcorder in free(): error: chunk is already free
> Abort trap (core dumped)

Core dump from double-free can be avoided by not free()'ing req_list
and prov_list in iterations where it's already in progress. It doesn't
solve circular dependency itself, though.

Index: sbin/rcorder/rcorder.c
===================================================================
RCS file: /home/source/freebsd/cvs/src/sbin/rcorder/rcorder.c,v
retrieving revision 1.1.1.2
diff -u -r1.1.1.2 rcorder.c
--- sbin/rcorder/rcorder.c	21 Jun 2002 15:56:16 -0000	1.1.1.2
+++ sbin/rcorder/rcorder.c	17 Sep 2004 05:51:24 -0000
_at__at_ -723,8 +723,8 _at__at_
 do_file(fnode)
 	filenode *fnode;
 {
-	f_reqnode *r, *r_tmp;
-	f_provnode *p, *p_tmp;
+	f_reqnode *r, *r_next;
+	f_provnode *p, *p_next;
 	provnode *pnode;
 	int was_set;	
 
_at__at_ -748,22 +748,17 _at__at_
 	 * for each requirement of fnode -> r
 	 *	satisfy_req(r, filename)
 	 */
-	r = fnode->req_list;
-	while (r != NULL) {
-		r_tmp = r;
+	for (r = fnode->req_list; r != NULL; r = r_next) {
+		r_next = r->next;
 		satisfy_req(r, fnode->filename);
-		r = r->next;
-		free(r_tmp);
 	}
-	fnode->req_list = NULL;
 
 	/*
 	 * for each provision of fnode -> p
 	 *	remove fnode from provision list for p in hash table
 	 */
-	p = fnode->prov_list;
-	while (p != NULL) {
-		p_tmp = p;
+	for (p = fnode->prov_list; p != NULL; p = p_next) {
+		p_next = p->next;
 		pnode = p->pnode;
 		if (pnode->next != NULL) {
 			pnode->next->last = pnode->last;
_at__at_ -771,11 +766,7 _at__at_
 		if (pnode->last != NULL) {
 			pnode->last->next = pnode->next;
 		}
-		free(pnode);
-		p = p->next;
-		free(p_tmp);
 	}
-	fnode->prov_list = NULL;
 
 	/* do_it(fnode) */
 	DPRINTF((stderr, "next do: "));
_at__at_ -792,8 +783,19 _at__at_
 	}
 
 	DPRINTF((stderr, "nuking %s\n", fnode->filename));
-	free(fnode->filename);
-	free(fnode);
+	if (!was_set) {
+		for (r = fnode->req_list; r != NULL; r = r_next) {
+			r_next = r->next;
+			free(r);
+		}
+		for (p = fnode->prov_list; p != NULL; p = p_next) {
+			p_next = p->next;
+			free(p->pnode);
+			free(p);
+		}
+		free(fnode->filename);
+		free(fnode);
+	}
 }
 
 void