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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Yq9obvFIv8LjAAvg@mit.edu>
Date:   Sun, 19 Jun 2022 14:18:22 -0400
From:   "Theodore Ts'o" <tytso@....edu>
To:     Liu Peibao <liupeibao@....com>
Cc:     adilger.kernel@...ger.ca, linux-ext4@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/2] ext4: page-io: use 'unsigned int' to bare use of
 'unsigned'

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