Re: [PATCH 02/20] btrfs: handle invalid range and start in merge_extent_mapping()

David Sterba
2024-01-25T16:07:58Z

On Thu, Jan 25, 2024 at 02:23:03PM +1030, Qu Wenruo wrote:
>
>
> On 2024/1/25 07:47, David Sterba wrote:
> > Turn a BUG_ON to a properly handled error and update the error message
> > in the caller. It is expected that @em_in and @start passed to
> > btrfs_add_extent_mapping() overlap. Besides tests, the only caller
> > btrfs_get_extent() makes sure this is true.
> >
> > Signed-off-by: David Sterba <dsterba@suse.com>
> > ---
> > fs/btrfs/extent_map.c | 9 +++++----
> > 1 file changed, 5 insertions(+), 4 deletions(-)
> >
> > diff --git a/fs/btrfs/extent_map.c b/fs/btrfs/extent_map.c
> > index f170e7122e74..ac5e366d57b2 100644
> > --- a/fs/btrfs/extent_map.c
> > +++ b/fs/btrfs/extent_map.c
> > @@ -539,7 +539,8 @@ static noinline int merge_extent_mapping(struct extent_map_tree *em_tree,
> > u64 end;
> > u64 start_diff;
> >
> > - BUG_ON(map_start < em->start || map_start >= extent_map_end(em));
> > + if (map_start < em->start || map_start >= extent_map_end(em))
> > + return -EINVAL;
>
> Shouldn't we go -EUCLEAN?
>
> This is not something we really expect to hit, as it normally means
> something wrong with the extent map.

The logic I used here is that it's a simple helper and it verifies the
arguments. If they're invalid then it's EINVAL. EUCLEAN needs some
interpretation like in the other patches unexpectedly finding an item,
it's up to the caller.