2023-07-07 11:25:46

by Lu Hongfei

Subject: [PATCH] fs/udf: Change the type of blocksize from 'int' to 'unsigned int' in udf_discard_prealloc

The return value type of i_blocksize() is 'unsigned int', so the
type of blocksize has been modified from 'int' to 'unsigned int'
to ensure data type consistency.

Signed-off-by: Lu Hongfei <[email protected]>
---
fs/udf/truncate.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/udf/truncate.c b/fs/udf/truncate.c
index a686c10fd709..c80dfcc583f6 100644
--- a/fs/udf/truncate.c
+++ b/fs/udf/truncate.c
@@ -123,7 +123,7 @@ void udf_discard_prealloc(struct inode *inode)
uint64_t lbcount = 0;
int8_t etype = -1;
struct udf_inode_info *iinfo = UDF_I(inode);
- int bsize = i_blocksize(inode);
+ unsigned int bsize = i_blocksize(inode);

if (iinfo->i_alloc_type == ICBTAG_FLAG_AD_IN_ICB ||
ALIGN(inode->i_size, bsize) == ALIGN(iinfo->i_lenExtents, bsize))
--
2.39.0

2023-07-07 12:30:59

by Jan Kara

Subject: Re: [PATCH] fs/udf: Change the type of blocksize from 'int' to 'unsigned int' in udf_discard_prealloc

On Fri 07-07-23 19:07:52, Lu Hongfei wrote:
> The return value type of i_blocksize() is 'unsigned int', so the
> type of blocksize has been modified from 'int' to 'unsigned int'
> to ensure data type consistency.
>
> Signed-off-by: Lu Hongfei <[email protected]>

Thanks for the patch but I'm sorry but how does this make a difference?
Blocksize is a small positive integer (512-64k). So it doesn't matter
whether you store it in int or unsigned it. I agree sometimes there are
nasty issues in C with signed comparisons, sign extension and other complex
logic and there it is beneficial to really be sure to match signedness etc.
to avoid subtle issues. But in this particular case I don't see the
point so I'd just keep the code as is... But please tell me if you see some
readability or other benefit in this change.

Honza

> ---
> fs/udf/truncate.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/udf/truncate.c b/fs/udf/truncate.c
> index a686c10fd709..c80dfcc583f6 100644
> --- a/fs/udf/truncate.c
> +++ b/fs/udf/truncate.c
> @@ -123,7 +123,7 @@ void udf_discard_prealloc(struct inode *inode)
> uint64_t lbcount = 0;
> int8_t etype = -1;
> struct udf_inode_info *iinfo = UDF_I(inode);
> - int bsize = i_blocksize(inode);
> + unsigned int bsize = i_blocksize(inode);
>
> if (iinfo->i_alloc_type == ICBTAG_FLAG_AD_IN_ICB ||
> ALIGN(inode->i_size, bsize) == ALIGN(iinfo->i_lenExtents, bsize))
> --
> 2.39.0
>
--
Jan Kara <[email protected]>
SUSE Labs, CR