Re: [PATCH 3/3] btrfs: do proper error handling in btrfs_insert_xattr_item

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Feb 21, 2017 at 09:39:05PM -0800, Liu Bo wrote:
> On Mon, Feb 20, 2017 at 07:25:06PM +0100, David Sterba wrote:
> > The space check in btrfs_insert_xattr_item is duplicated in it's caller
> > (do_setxattr) so we won't hit the BUG_ON. Continuing without any check
> > could be disasterous so turn it to a proper error handling.
> > 
> > Signed-off-by: David Sterba <dsterba@xxxxxxxx>
> > ---
> >  fs/btrfs/dir-item.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/fs/btrfs/dir-item.c b/fs/btrfs/dir-item.c
> > index 724504a2d7ac..640801082533 100644
> > --- a/fs/btrfs/dir-item.c
> > +++ b/fs/btrfs/dir-item.c
> > @@ -80,7 +80,8 @@ int btrfs_insert_xattr_item(struct btrfs_trans_handle *trans,
> >  	struct extent_buffer *leaf;
> >  	u32 data_size;
> >  
> > -	BUG_ON(name_len + data_len > BTRFS_MAX_XATTR_SIZE(root->fs_info));
> > +	if (name_len + data_len > BTRFS_MAX_XATTR_SIZE(root->fs_info))
> > +		return -ENOSPC;
> >
> 
> Besides making it silent, how about adding a ASSERT to cry out?
> (Although currently we'd never come into this case.)

I don't think we need the assert, the caller is supposed to handle the
error. In this case it's validation of input parameters, that could
possibly happen as the function is not static.
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Filesystem Development]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux