On Mon, Aug 30, 2021 at 04:05:28PM +0300, Nikolay Borisov wrote:

quoted

@@ -6137,35 +6136,19 @@ static int btrfs_real_readdir(struct file *file, struct dir_context *ctx)
 	key.offset = ctx->pos;
 	key.objectid = btrfs_ino(BTRFS_I(inode));
 
-	ret = btrfs_search_slot(NULL, root, &key, path, 0, 0);
-	if (ret < 0)
-		goto err;
-
-	while (1) {
+	btrfs_for_each_slot(root, &key, &found_key, path, iter_ret) {

I don't think it's necessary to use iter_ret, instead you can use ret
directly. Because if either btrfs_search_slot return an error or
btrfs_valid_slot then ret would be set to the respective return value
and the body of the loop won't be executed at all, no?

Yeah thre's no reason to add another variable in this case. As long as
the loop body does not use ret internally, then reusing ret is fine.

The point of having an explicit return value for the iterator is to be
able to read the reason of failure after the iterator scope ends, so it
can't be defined inside. We'd need to be careful to make sure that the
iterator 'ret' is never used inside the body so that could be also
useful to put to the documentation. I think a coccinelle script can be
also useful to catch such things.