> The use of ?: here is a bit strange. How about: > > return algorithm < COMPRESS_MAX && f2fs_cops[algorithm] != NULL; >
Ack > Likewise, EINVAL tends to be over-used, which makes it ambiguous. Maybe use > ENOPKG for the case where algorithm < COMPRESS_MAX but the algorithm wasn't > compiled into the kernel? That would be similar to how opening an encrypted > file fails with ENOPKG when the encryption algorithm isn't available. Ack > How about EBUSY for f2fs_is_mmap_file(inode) || get_dirty_pages(inode), > and EFBIG for inode->i_size != 0? Ack Thanks~! 2020년 10월 22일 (목) 오후 1:26, Eric Biggers <[email protected]>님이 작성: > > On Thu, Oct 22, 2020 at 12:58:48PM +0900, Daeho Jeong wrote: > > diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c > > index 7895186cc765..3b58a41223f8 100644 > > --- a/fs/f2fs/compress.c > > +++ b/fs/f2fs/compress.c > > @@ -514,6 +514,11 @@ bool f2fs_is_compress_backend_ready(struct inode > > *inode) > > return f2fs_cops[F2FS_I(inode)->i_compress_algorithm]; > > } > > > > +bool f2fs_is_compress_algorithm_ready(unsigned char algorithm) > > +{ > > + return algorithm >= COMPRESS_MAX ? false : f2fs_cops[algorithm]; > > +} > > The use of ?: here is a bit strange. How about: > > return algorithm < COMPRESS_MAX && f2fs_cops[algorithm] != NULL; > > > + if (option.log_cluster_size < MIN_COMPRESS_LOG_SIZE || > > + option.log_cluster_size > MAX_COMPRESS_LOG_SIZE || > > + !f2fs_is_compress_algorithm_ready(option.algorithm)) > > + return -EINVAL; > > Likewise, EINVAL tends to be over-used, which makes it ambiguous. Maybe use > ENOPKG for the case where algorithm < COMPRESS_MAX but the algorithm wasn't > compiled into the kernel? That would be similar to how opening an encrypted > file fails with ENOPKG when the encryption algorithm isn't available. > > > + if (f2fs_is_mmap_file(inode) || > > + get_dirty_pages(inode) || inode->i_size) { > > + ret = -EINVAL; > > + goto out; > > + } > > How about EBUSY for f2fs_is_mmap_file(inode) || get_dirty_pages(inode), > and EFBIG for inode->i_size != 0? > > - Eric
