Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Thu, 13 Jul 2023 09:41:29 +0100
From:   Simon Horman <simon.horman@corigine.com>
To:     yunchuan <yunchuan@nfschina.com>
Cc:     mostrows@earthlink.net, davem@davemloft.net, edumazet@google.com,
        kuba@kernel.org, pabeni@redhat.com, xeb@mail.ru,
        netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
        kernel-janitors@vger.kernel.org
Subject: Re: [PATCH net-next v2 03/10] net: ppp: Remove unnecessary (void*)
 conversions
Message-ID: <ZK+4udwfawcJq5qC@corigine.com>
References: <ZK2Wf4man0H4e9Zz@corigine.com>
 <284c0843-b749-00c8-95bc-6a0b220d9421@nfschina.com>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <284c0843-b749-00c8-95bc-6a0b220d9421@nfschina.com>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?fWRDjfrQl7VWvqauNPwwZ4n/+s9ZZ57/wBQofTR8Xu4hv+37dzKZBYBRaRxp?=
 =?us-ascii?Q?QlLp45nCiKI0bneCL6BsjoWBZqrdgpq//3icgP65g7f9xm8RZEyZy2uGJzA6?=
 =?us-ascii?Q?sgGMH/sdc1bcXDQc5+co72Xxv32OLMtOMqPhP8BfFmKKnjvo1o+FJA5wVZUz?=
 =?us-ascii?Q?8URf9wJs7UxNq10A4e8zqt3OKIZadPvGy7WitoubVmy53ITXr48CeAXfSlee?=
 =?us-ascii?Q?xsL3KrR/yVHvURDNM6iOJdV+rA48SIJ+QOFLD+KUPlsAOo96NxJx+2bx4czq?=
 =?us-ascii?Q?PBCtOLORKJ9X6SYa/VAGEG5sL45jMWRfKWVux7ZkX+3kpjA2fzW7r+j/Wv8l?=
 =?us-ascii?Q?vRu4YrdUCT8xvzF+HRMShzF2ZsSVR3lCvBUJYuB5WP5Ysp07RMh+SI8j2sDW?=
 =?us-ascii?Q?ElgsUS1bfR6pZyfJ875VmJb0b0nH25W0oYcomCszT5A+OBAs0laQ5nE3/4ie?=
 =?us-ascii?Q?jn448QAbsgN5hzfuEsZt/2s19MHTW+u4v8fwnNGEGVlFqLd1NVuu/gDWhfnb?=
 =?us-ascii?Q?+7zUweB3hcsum9ibEAyMRhD8rtjv5NlJ+o13aHQgIQ7Y4R/QTx60ETXEDRMA?=
 =?us-ascii?Q?430i1mj8lJA+OaWVKVtba4cATXH/6xDCTArwqkk5LN3qFHbuyhVUChr5X3cm?=
 =?us-ascii?Q?sS9/qXi+NetcUVaJwxkyWqMXeKVaObk9VsSWb9Xq6TfQRoWmVpCV135SJ/C+?=
 =?us-ascii?Q?IYKfyp/A8cuG6e4SxIpZ/kGVMFK9bLfPYH2EEoJjIKNMV3Fj/MbkmPeEFQDr?=
 =?us-ascii?Q?XauCws4mbe9u/t48pahMgP22I+KBBYzjWzAefJlDwcXA+wTd4GLFk9+IIbju?=
 =?us-ascii?Q?DKIKpN0Vbt9poxrXddl5LjmqxTmAPnAH5CqsQd5qenTU7nShMEm7wT/XCBm6?=
 =?us-ascii?Q?zK/NmumbMjuz///fUT66YEvniLf1Bri0T722CAD+gsytFf2h1b6bLSel/X9A?=
 =?us-ascii?Q?Kpt4cKuX0Qp355T878dapfbN5pyUoR3Yts+aK2CEppjS2qBhd5gW4igzG/Lx?=
 =?us-ascii?Q?+HHuqYezvvoSpaBDkPtzGBlwmvtP3ghgLUDzn31xYMu6m8IVl9npOSfDZAXX?=
 =?us-ascii?Q?yOaH+crPk3juYCheylHwIJiqa7VmPEMu8wd2Z/hMRDl55v8bOZzrq5jI1ZOi?=
 =?us-ascii?Q?MinDXMHhwyDrc95++ozlXuPDZEeeAmSyUU4UsyCbAbCiN9SVlkXIBQgat3oc?=
 =?us-ascii?Q?6CWFncvze/D2AFxJq1hLLrmnvwESQKWO+3o+FrJbPKFEIlpyQtBQuZK19wnJ?=
 =?us-ascii?Q?HOBFFT40TjmI3u5+XdD9GOAil3BldCk1mcN/4DnBODkJHMgJHhCylkP6h/mt?=
 =?us-ascii?Q?xQjkZpOfL4fU/5vpLI2kZC89HhVbdzH7wq1AbnwIysg5zdl1QEISdqkHSV8p?=
 =?us-ascii?Q?AMZNCXcLhTPdp6QRVN7mDz8n2xiVfuupaL5R6u3/HgVs3ZX/mnENV2jxKCoD?=
 =?us-ascii?Q?yaSMO3z6Yytfm44SsPpBi9PuCDuPZ2t37/zcEeQ3eXWZt1yXQHLR+Zzh74aG?=
 =?us-ascii?Q?ZoaaC6Ip9/KRMC+iKrXSG53AU4jeqle2NwSoiahEBJkQZkCiyaTIGfY0PWjN?=
 =?us-ascii?Q?RghWX9yFRu/vZaNuyKM8THRP9I4UmocRUFDn3iRCVSovq21I2HMyBfsWgRAs?=
 =?us-ascii?Q?Tw=3D=3D?=
X-OriginatorOrg: corigine.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 2d898344-7590-4eba-5d63-08db837cf7df
X-MS-Exchange-CrossTenant-AuthSource: PH0PR13MB4842.namprd13.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 13 Jul 2023 08:41:36.3442
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: fe128f2c-073b-4c20-818e-7246a585940c
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: XkUtT/gzSZqeOvzjFOPQEnxCzpdyyX2BSYjjOn+yJIhPHbhENZ7AXN1l+RlHyFC8gTc6Zxi15wP4VHAJEj7u0lNWuqKQQqkoKaHdJRS4z/Q=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CO1PR13MB4903
X-Spam-Status: No, score=-1.9 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,RCVD_IN_DNSWL_BLOCKED,RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,
        SPF_PASS,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no
        version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Wed, Jul 12, 2023 at 09:44:40AM +0800, yunchuan wrote:
> On 2023/7/12 01:50, Simon Horman wrote:
> > > -	struct sock *sk = (struct sock *)chan->private;
> > > +	struct sock *sk = chan->private;
> > >   	struct pppox_sock *po = pppox_sk(sk);
> > >   	struct net_device *dev = po->pppoe_dev;
> > Hi,
> > 
> > Please don't break reverse xmas tree ordering - longest line to shortest -
> > of local variable declarations in Networking code.
> 
> Hi,
> 
> This can't be reversed because it depends on the first declaration.
> Should I change it like this?
> 
> -	struct sock *sk = (struct sock *)chan->private;
> - 	struct pppox_sock *po = pppox_sk(sk);
> +	struct pppox_sock *po = pppox_sk(chan->private);
>  	struct net_device *dev = po->pppoe_dev;
> +	struct sock *sk = chan->private;
> 
> But this seems to be bad. As the advice of Andrew[1] and Dan[2]:
> 
> "
> 
> When dealing with existing broken reverse Christmas tree, please don't
> make it worse with a change. But actually fixing it should be in a
> different patch.
> 
> We want patches to be obviously correct. By removing the cast and
> moving variables around, it is less obvious it is correct, than having
> two patches.
> 
> "

Thanks, I agree this is a good approach.