Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Fri, 7 Jul 2023 14:19:47 +0200
From:   Jan Kara <jack@suse.cz>
To:     Lu Hongfei <luhongfei@vivo.com>
Cc:     Jan Kara <jack@suse.com>, linux-kernel@vger.kernel.org,
        opensource.kernel@vivo.com
Subject: Re: [PATCH] fs/udf: Change the type of blocksize from 'int' to
 'unsigned int' in udf_discard_prealloc
Message-ID: <20230707121947.sa2l3y3b5d2kwmdj@quack3>
References: <20230707110752.13436-1-luhongfei@vivo.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20230707110752.13436-1-luhongfei@vivo.com>
Precedence: bulk

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 <luhongfei@vivo.com>

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 <jack@suse.com>
SUSE Labs, CR