delete obsolete function btrfs_print_tree()

Dan Carpenter's static checker recently found missing IS_ERR handling in print-tree.c:btrfs_print_tree(). While looking into this I found that this function is no longer called anywhere and was moved to btrfs-progs long ago. It can simply be removed.

---
 fs/btrfs/print-tree.c | 38 --------------------------------------
 fs/btrfs/print-tree.h |  1 -
 2 files changed, 39 deletions(-)

index 147dc6c..dc28db8 100644
@@ -328,41 +328,3 @@ void btrfs_print_leaf(struct btrfs_root *root, struct extent_buffer *l)
 		};
 	}
 }
-
-void btrfs_print_tree(struct btrfs_root *root, struct extent_buffer *c)
-{
-	int i; u32 nr;
-	struct btrfs_key key;
-	int level;
-
-	if (!c)
-		return;
-	nr = btrfs_header_nritems(c);
-	level = btrfs_header_level(c);
-	if (level == 0) {
-		btrfs_print_leaf(root, c);
-		return;
-	}
-	btrfs_info(root->fs_info, "node %llu level %d total ptrs %d free spc %u",
-		btrfs_header_bytenr(c), level, nr,
-		(u32)BTRFS_NODEPTRS_PER_BLOCK(root) - nr);
-	for (i = 0; i < nr; i++) {
-		btrfs_node_key_to_cpu(c, &key, i);
-		printk(KERN_INFO "\tkey %d (%llu %u %llu) block %llu\n",
-		       i, key.objectid, key.type, key.offset,
-		       btrfs_node_blockptr(c, i));
-	}
-	for (i = 0; i < nr; i++) {
-		struct extent_buffer *next = read_tree_block(root,
-					btrfs_node_blockptr(c, i),
-					btrfs_node_ptr_generation(c, i));
-		if (btrfs_is_leaf(next) &&
-		   level != 1)
-			BUG();
-		if (btrfs_header_level(next) !=
-		       level - 1)
-			BUG();
-		btrfs_print_tree(root, next);
-		free_extent_buffer(next);
-	}
-}

index 7faddfa..9dd56b9 100644
@@ -19,5 +19,4 @@
 #ifndef __PRINT_TREE_
 #define __PRINT_TREE_
 void btrfs_print_leaf(struct btrfs_root *root, struct extent_buffer *l);
-void btrfs_print_tree(struct btrfs_root *root, struct extent_buffer *c);
 #endif

2.7.4

I'm not sure, the function could be used for debugging, and it's hard to say if we'll ever need it. Printing the whole tree to the system log would produce a lot of text so some manual filtering would be required, the function could serve as a template.

The function is not that big that it would save bytes, but putting it under the debug config would help a bit.

..but is it? So far nobody has complained.

The original problem of missing error handling from btrfs_read_tree_block() remains as well. I don't remember if that also was true for the btrfs-progs counterpart, but in in any case I didn't really know what to do there. Print an error? silently ignore the stripe? abort? When I realized that the function was not called anywhere, deleting it seemed more effective.

Under what circumstances would the in-kernel function be more practical or useful than the userland tool? It does the same, won't disturb or wedge the kernel further, is up-to-date and can be scripted. I agree that in-place filtering (while iterating) would be nice to have, but that's also a whole different problem and would IMHO also be better suited for userland.

When in doubt cut it out.

Holger

I will complain.
I use this function frequently during development, and there's a good reason to use it
instead of the user space tool btrfs-debug-tree.

The user land tool requires the btree nodes to be on disk. With the in kernel function we can print nodes
that are not yet on disk, very useful during development.

So no, we should not delete it in my opinion. It's not serious if it doesn't have all the proper error handling
and etc, it's just something for debugging purposes.

It does the same, won't disturb

When in doubt leave it alone.

Filipe David Manana,

"Reasonable men adapt themselves to the world.
 Unreasonable men adapt the world to themselves.
 That's why all progress depends on unreasonable men."

Good to know, that's why I asked. Printing unwritten extents makes sense.

-h

I'm slowly trying to remove static checker warnings so that we can detect real bugs. People sometimes leave little messages for me in their code because they know I will review the new warning:

	foo = kmalloc();
	/* error handling deliberately left out */

It make me quite annoyed because it's like "Oh. No if we added error handling that would take 40 extra bytes of memory! Such a waste!" But we could use instead use___GFP_NOFAIL instead. Or BUG_ON(!foo)". I have gotten distracted. What was the question again?

regards,
dan carpenter