On Tue, Apr 16, 2013 at 03:38:28PM +0200, David Sterba wrote: > On Tue, Apr 16, 2013 at 05:20:28PM +0800, Liu Bo wrote: > > We need such a sanity check for wrong start when we defrag a file, otherwise, > > even with a wrong start that's larger than file size, we can end up changing > > not only inode's force compress flag but also FS's incompat flags. > > The range->start check is good, but why are you worried about the > incompat flag? LZO support has been around for more than 2 years. As the code of setting LZO incompat flags is just there, so I take it as a side effect. Well, I'm not worried now :) > > > --- a/fs/btrfs/ioctl.c > > +++ b/fs/btrfs/ioctl.c > > @@ -1152,8 +1152,11 @@ int btrfs_defrag_file(struct inode *inode, struct file *file, > > u64 new_align = ~((u64)128 * 1024 - 1); > > struct page **pages = NULL; > > > > - if (extent_thresh == 0) > > - extent_thresh = 256 * 1024; > > + if (isize == 0) > > + return 0; > > + > > + if (range->start >= isize) > > + return -EINVAL; > ... > > - if (isize == 0) > > - return 0; > > + if (extent_thresh == 0) > > + extent_thresh = 256 * 1024; > > That's a more logical order of the checks, good. > > Reviewed-by: David Sterba <dsterba@xxxxxxx> Thanks for the quick review! thanks, liubo -- 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
