On Tue, Jul 21, 2020 at 06:29:31PM +0800, Qu Wenruo wrote:
> On 2020/7/21 下午5:58, David Sterba wrote:
> > On Tue, Jul 21, 2020 at 07:51:00AM +0800, Qu Wenruo wrote:
> >>
> >>
> >> On 2020/7/21 上午12:09, David Sterba wrote:
> >>> On Mon, Jul 20, 2020 at 08:51:08PM +0800, Qu Wenruo wrote:
> >>>> --- a/convert/source-ext2.c
> >>>> +++ b/convert/source-ext2.c
> >>>> @@ -87,7 +87,8 @@ static int ext2_open_fs(struct btrfs_convert_context *cctx, const char *name)
> >>>> cctx->fs_data = ext2_fs;
> >>>> cctx->blocksize = ext2_fs->blocksize;
> >>>> cctx->block_count = ext2_fs->super->s_blocks_count;
> >>>> - cctx->total_bytes = ext2_fs->blocksize * ext2_fs->super->s_blocks_count;
> >>>> + cctx->total_bytes = (u64)ext2_fs->blocksize *
> >>>> + (u64)ext2_fs->super->s_blocks_count;
> >>>
> >>> Do you need to cast both? Once one of the types is wide enough for the
> >>> result, there should be no loss.
> >>>
> >> I just want to be extra safe.
> >
> > Typecasts in code raise questions why are they needed, 'to be extra'
> > safe is not a good reason. One typecast in multiplication/shifts is a
> > common pattern to widen the result but two look more like lack of
> > understanding of the integer promotion rules.
> >
>
> My point here is, I don't want the reviewers or new contributors to
> bother about the promotion rules at all.
Reviewers and contributors need to understand the rules of C.
> They only need to know that using blocksize and blocks_count directly to
> do multiply would lead to overflow.
>
> Other details like whether the multiply follows the highest factor or
> the left operator or the right operator, shouldn't be the point and we
> don't really need to bother.
This is like suggesting to use brackets on every int-expression
like ((5*3)+7) because the precedere rules are too complex.
Do you think that writing ((5*3)+7) is "extra safe"?
> Thus casting both would definitely be right, without the need to refer
> to the complex rule book, thus save the reviewer several minutes.
I don't think so.
--
ciao -
Stefan
