Re: [PATCH 1/2] ext4: page-io: use 'unsigned int' to bare use of 'unsigned'

[Liu Peibao]

On 6/20/22 2:18 AM, Theodore Ts'o wrote:
> On Sun, Jun 19, 2022 at 11:21:27AM +0800, Liu Peibao wrote:
> > Thanks for your reply. What I want do to is rename some temporary variables
> > in the patch2 and when I make the patch, there are the checkpatch warnings.
> >  From the point of view "one patch do one thing", I split the modification
> > into two patches. Thanks!
>
> I didn't really see the poiont of renaming the temporary variables,
> either.
>
> In this particular case basically only used to avoid line lengths from
> exceeding ~72 characters, and requiring a line wrap, and bio_start and
> bio_end is used only in one place in the code block below.
>
> Is it _really_ all that confusing whether they are named
> bio_{start,end} instead of bvec_{start,end}?
>
> If I was writing that code from scratch, I might have just used start
> and end without any prefixes.  And as far as "only have a patch do one
> thing at a time", this doesn't apply to checkpatch fixes.
>
> The basic motivation behind "no checkpatch-only fixes" is that it
> tends to introduce code churn which makes interpreting information
> from "git blame" more difficult; and so therefore the costs exceed the
> extremely marginal benefits of fixing most checkpatch complaints.  So
> making a _patch_ be checkpatch clean, whether it's modifying existing
> code or writing new code, is fine, since you're making a subtantive
> change to the code, so this is as good a time as any to fix up tiny
> nits such as checkpatch complaints.
>
> But the idea behind "no unnecessary code churn since it ruins git
> blame and could potentially induce future patch conflicts" also
> applies to renaming variables.  The benefits are very minor, and they
> don't outweigh the costs.
>
> 						- Ted

Got it! Thanks for your detailed and comprehensive explanation!

Best Regards,
Peibao