[PATCH V2] xfs_repair: fix record_allocation list manipulation

Eric Sandeen sandeen at sandeen.net
Sat Sep 19 09:42:04 CDT 2009


clang found this one too as a "Dead assignment"

Unless my pointer-fu is totally messed up, this function
was never actually updating the list head.

This would mean that the later free_allocations() calls in
incore_ext_teardown() and free_rt_dup_extent_tree() don't
actually free any items, and therefore leak memory.

V2: now with correct pointer-fu.

Signed-off-by: Eric Sandeen <sandeen at sandeen.net>
---

(We could use Jeff's doubly-linked list patch too, though we
really don't need both pointers, and there are still other
singly-linked lists throughout repair...)

clang record:

http://sandeen.net/clang/xfsprogs/2009-09-09-1/report-1Jnl15.html#EndPath

diff --git a/repair/incore.c b/repair/incore.c
index 84626c9..0fd0e89 100644
--- a/repair/incore.c
+++ b/repair/incore.c
@@ -30,10 +30,10 @@
  * if set to NULL if empty.
  */
 void
-record_allocation(ba_rec_t *addr, ba_rec_t *list)
+record_allocation(ba_rec_t *addr, ba_rec_t **list)
 {
-	addr->next = list;
-	list = addr;
+	addr->next = *list;
+	*list = addr;
 
 	return;
 }
diff --git a/repair/incore.h b/repair/incore.h
index 1f0f45a..4f90dd6 100644
--- a/repair/incore.h
+++ b/repair/incore.h
@@ -33,7 +33,7 @@ typedef struct ba_rec  {
 	struct ba_rec	*next;
 } ba_rec_t;
 
-void			record_allocation(ba_rec_t *addr, ba_rec_t *list);
+void			record_allocation(ba_rec_t *addr, ba_rec_t **list);
 void			free_allocations(ba_rec_t *list);
 
 /*
diff --git a/repair/incore_ext.c b/repair/incore_ext.c
index a2acbf4..90d2074 100644
--- a/repair/incore_ext.c
+++ b/repair/incore_ext.c
@@ -120,7 +120,7 @@ mk_extent_tree_nodes(xfs_agblock_t new_startblock,
 			do_error(
 			_("couldn't allocate new extent descriptors.\n"));
 
-		record_allocation(&rec->alloc_rec, ba_list);
+		record_allocation(&rec->alloc_rec, &ba_list);
 
 		new = &rec->extents[0];
 
@@ -678,7 +678,7 @@ mk_rt_extent_tree_nodes(xfs_drtbno_t new_startblock,
 			do_error(
 			_("couldn't allocate new extent descriptors.\n"));
 
-		record_allocation(&rec->alloc_rec, rt_ba_list);
+		record_allocation(&rec->alloc_rec, &rt_ba_list);
 
 		new = &rec->extents[0];
 




More information about the xfs mailing list