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 5580AC433EF for ; Sun, 24 Apr 2022 23:26:22 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S239895AbiDXX3R (ORCPT ); Sun, 24 Apr 2022 19:29:17 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:38826 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S233081AbiDXX3O (ORCPT ); Sun, 24 Apr 2022 19:29:14 -0400 Received: from angie.orcam.me.uk (angie.orcam.me.uk [IPv6:2001:4190:8020::34]) by lindbergh.monkeyblade.net (Postfix) with ESMTP id DA8536A043; Sun, 24 Apr 2022 16:26:11 -0700 (PDT) Received: by angie.orcam.me.uk (Postfix, from userid 500) id 901EF92009C; Mon, 25 Apr 2022 01:26:10 +0200 (CEST) Received: from localhost (localhost [127.0.0.1]) by angie.orcam.me.uk (Postfix) with ESMTP id 8248092009B; Mon, 25 Apr 2022 00:26:10 +0100 (BST) Date: Mon, 25 Apr 2022 00:26:10 +0100 (BST) From: "Maciej W. Rozycki" To: Andrew Lunn cc: Wan Jiabing , "David S. Miller" , Jakub Kicinski , Paolo Abeni , netdev@vger.kernel.org, linux-kernel@vger.kernel.org, kael_w@yeah.net Subject: Re: [PATCH] FDDI: defxx: simplify if-if to if-else In-Reply-To: Message-ID: References: <20220424092842.101307-1-wanjiabing@vivo.com> User-Agent: Alpine 2.21 (DEB 202 2017-01-01) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, 25 Apr 2022, Andrew Lunn wrote: > > NAK. The first conditional optionally sets `bp->mmio = false', which > > changes the value of `dfx_use_mmio' in some configurations: > > > > #if defined(CONFIG_EISA) || defined(CONFIG_PCI) > > #define dfx_use_mmio bp->mmio > > #else > > #define dfx_use_mmio true > > #endif > > Which is just asking for trouble like this. > > Could i suggest dfx_use_mmio is changed to DFX_USE_MMIO to give a hint > something horrible is going on. There's legacy behind it, `dfx_use_mmio' used to be a proper variable and references were retained not to obfuscate the changes that ultimately led to the current arrangement. I guess at this stage it could be changed to a function-like macro or a static inline function taking `bp' as the argument. > It probably won't stop the robots finding this if (x) if (!x), but > there is a chance the robot drivers will wonder why it is upper case. Well, blindly relying on automation is bound to cause trouble. There has to be a piece of intelligence signing the results off at the end. And there's nothing wrong with if (x) if (!x) in the first place; any sane compiler will produce reasonable output from it. Don't fix what ain't broke! And watch out for volatiles! Maciej