On Mon, Dec 17, 2018 at 10:36:01AM +0200, Nikolay Borisov wrote:
> The code which executes if path->slots[0] points to an item not
> belonging to the inode we are interested in or is not na extent at all
> could really trigger if btrfs_lookup_file_extent returned > 0 and
> subsequently path->slots[0] was decremented. Make this explicit by
> moving the respective code in the correct if branch. No functional
> changes.
> 
> Signed-off-by: Nikolay Borisov <[email protected]>
> ---
>  fs/btrfs/inode.c | 36 ++++++++++++++++++++----------------
>  1 file changed, 20 insertions(+), 16 deletions(-)
> 
> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> index 511d3b314af2..e8284cd0a122 100644
> --- a/fs/btrfs/inode.c
> +++ b/fs/btrfs/inode.c
> @@ -6805,22 +6805,26 @@ struct extent_map *btrfs_get_extent(struct 
> btrfs_inode *inode,
>               if (path->slots[0] == 0)
>                       goto not_found;
>               path->slots[0]--;
> -     }
> -
> -     leaf = path->nodes[0];
> -     item = btrfs_item_ptr(leaf, path->slots[0],
> -                           struct btrfs_file_extent_item);
> -     btrfs_item_key_to_cpu(leaf, &found_key, path->slots[0]);
> -     if (found_key.objectid != objectid ||
> -         found_key.type != BTRFS_EXTENT_DATA_KEY) {
> -             /*
> -              * If we backup past the first extent we want to move forward
> -              * and see if there is an extent in front of us, otherwise we'll
> -              * say there is a hole for our whole search range which can
> -              * cause problems.
> -              */
> -             extent_end = start;
> -             goto next;
> +             leaf = path->nodes[0];
> +             item = btrfs_item_ptr(leaf, path->slots[0],
> +                                   struct btrfs_file_extent_item);
> +             btrfs_item_key_to_cpu(leaf, &found_key, path->slots[0]);
> +             if (found_key.objectid != objectid ||
> +                 found_key.type != BTRFS_EXTENT_DATA_KEY) {
> +                     /*
> +                      * If we backup past the first extent we want to move
> +                      * forward and see if there is an extent in front of us,
> +                      * otherwise we'll say there is a hole for our whole
> +                      * search range which can cause problems.
> +                      */
> +                     extent_end = start;
> +                     goto next;
> +             }
> +     } else {
> +             leaf = path->nodes[0];
> +             item = btrfs_item_ptr(leaf, path->slots[0],
> +                                   struct btrfs_file_extent_item);

This is repeating code and could be simplified ... to the original code.
I'm not sure this patch is an improvement.

Reply via email to