Hello, > On 03/29/13 14:42, Wang Shilong wrote: >> From: Wang Shilong <wangsl-f...@cn.fujitsu.com> >> >> Just remove the unnecessary check and assignment. >> >> Signed-off-by: Wang Shilong <wangsl-f...@cn.fujitsu.com> >> --- >> fs/btrfs/backref.c | 3 +-- >> 1 file changed, 1 insertion(+), 2 deletions(-) >> >> diff --git a/fs/btrfs/backref.c b/fs/btrfs/backref.c >> index 3ca413bb..e102b48 100644 >> --- a/fs/btrfs/backref.c >> +++ b/fs/btrfs/backref.c >> @@ -1499,7 +1499,7 @@ int iterate_extent_inodes(struct btrfs_fs_info >> *fs_info, >> if (ret) >> break; >> ULIST_ITER_INIT(&root_uiter); >> - while (!ret && (root_node = ulist_next(roots, &root_uiter))) { >> + while ((root_node = ulist_next(roots, &root_uiter))) { > > It doesn't look unnecessary at all to me. ret is set in the loop and > only checked in the while condition.
Yeah, you are right.. > >> pr_debug("root %llu references leaf %llu, data list " >> "%#llx\n", root_node->val, ref_node->val, >> (long long)ref_node->aux); >> @@ -1510,7 +1510,6 @@ int iterate_extent_inodes(struct btrfs_fs_info >> *fs_info, >> iterate, ctx); >> } >> ulist_free(roots); >> - roots = NULL; > > roots gets freed again later on. If you don't set it to NULL, it will > result in a double free. If we are in the loop, 'roots' will be reallocated again, if relocation in the find_all_roots() fails, 'roots' has been dealt in the find_all_roots(), and we have breaked out the loop. I don't know where a double may happen? Am i missing something? Thanks, Wang > > -Arne > >> } >> >> free_leaf_list(refs); >> > -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html