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] [day] [month] [year] [list]

Message-ID: <902000f3-7d9b-3115-0864-3ffa0f87d4d4@163.com>
Date:   Tue, 21 Jun 2022 22:28:56 +0800
From:   Liu Peibao <liupeibao@....com>
To:     Theodore Ts'o <tytso@....edu>
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 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