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 PHC | |
Open Source and information security mailing list archives
| ||
|
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
Powered by blists - more mailing lists