From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 4BD8AC433EF for ; Sun, 19 Jun 2022 18:18:43 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S235113AbiFSSSk (ORCPT ); Sun, 19 Jun 2022 14:18:40 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:43168 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231223AbiFSSSh (ORCPT ); Sun, 19 Jun 2022 14:18:37 -0400 Received: from outgoing.mit.edu (outgoing-auth-1.mit.edu [18.9.28.11]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 1F9CA2BFF; Sun, 19 Jun 2022 11:18:32 -0700 (PDT) Received: from cwcc.thunk.org (pool-173-48-118-63.bstnma.fios.verizon.net [173.48.118.63]) (authenticated bits=0) (User authenticated as tytso@ATHENA.MIT.EDU) by outgoing.mit.edu (8.14.7/8.12.4) with ESMTP id 25JIIMqs002746 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Sun, 19 Jun 2022 14:18:23 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=mit.edu; s=outgoing; t=1655662704; bh=SyUfwc+bUGSM/MSdWqbj2kBmTS/hxf15FkYmwwbBpAg=; h=Date:From:To:Cc:Subject:References:In-Reply-To; b=RcMcimfNR9ZLl5D7IJBNi/pYD6H8dwZOyJcIZLnrcdQDX5hd1U8Qo71yHsTabJ3Pe ED+SZS1Q5NmNgP0gzcp4ACtlEXOwJQPP/FULydujufs1D6a21JhyMV6N0PHFBUwQ7M gNHGeqF3VNSlh1vRYn4yvySN+EBz+HrqxhzUoBPbIKOTuUftdsF/iACifD1Exs3dsi qgT/0ORdYt8XS9AmnDxSiBoYVI453RdPuFuy/nnU1Y+IezObvF78dTYsFKK9vSlqmx W+GEXWRiWZrqNck5Imk+YQM+b64/S2E3jutRnWZmP68Aq6v/gSXLMRA+RZH/kF75Q/ LZyCNGllPcGnw== Received: by cwcc.thunk.org (Postfix, from userid 15806) id 4F65F15C431D; Sun, 19 Jun 2022 14:18:22 -0400 (EDT) Date: Sun, 19 Jun 2022 14:18:22 -0400 From: "Theodore Ts'o" To: Liu Peibao Cc: adilger.kernel@dilger.ca, linux-ext4@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH 1/2] ext4: page-io: use 'unsigned int' to bare use of 'unsigned' Message-ID: References: <20220518120137.2544-1-liupeibao@163.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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