xfs
[Top] [All Lists]

[PATCH V2] xfs_repair: fix record_allocation list manipulation

To: xfs-oss <xfs@xxxxxxxxxxx>
Subject: [PATCH V2] xfs_repair: fix record_allocation list manipulation
From: Eric Sandeen <sandeen@xxxxxxxxxxx>
Date: Sat, 19 Sep 2009 09:42:04 -0500
In-reply-to: <4AB300CC.5020707@xxxxxxxxxxx>
References: <4AB300CC.5020707@xxxxxxxxxxx>
User-agent: Thunderbird 2.0.0.23 (Macintosh/20090812)
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@xxxxxxxxxxx>
---

(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];


<Prev in Thread] Current Thread [Next in Thread>