From patchwork Tue Feb 6 13:40:35 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Julia Lawall X-Patchwork-Id: 10203115 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 16F3A605F3 for ; Tue, 6 Feb 2018 13:41:44 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 057FE28BCA for ; Tue, 6 Feb 2018 13:41:44 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 0397628BA1; Tue, 6 Feb 2018 13:41:43 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-4.2 required=2.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,RCVD_IN_DNSWL_MED autolearn=unavailable version=3.3.1 Received: from bombadil.infradead.org (bombadil.infradead.org [65.50.211.133]) (using TLSv1.2 with cipher AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 4C7E528C03 for ; Tue, 6 Feb 2018 13:41:36 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20170209; h=Sender: Content-Transfer-Encoding:Content-Type:Cc:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:MIME-Version:References:Message-ID: In-Reply-To:Subject:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=GVDDq9Z5tFYiWFrdVGxn4X6llB6W3KfmygeopL4ZtwY=; b=n9inYmvaPmYQTo g4UMq91+Nm5h0p+e3oHpVZUu2EdiouWas6f8FDcn2ig03DaWCzJeYO5CyzXn95WdYDfwlGnZ4aQPq 28QoemeUSMxeZ1E1myJOmZWqOxYRuWz8DhuD9p83Y+VidiEgpJdpQ+JXlDGJQwZTUR/rkWAGIccFw qUUdTFC376ZjI11EMRe1sYwLkF/+XuIpGMbAWDkaxO4FM5sw0+l3aHcqCuu4moORf6V+nN+uPVPP7 wwWE0k6kIMlfYPk4/ELmzMDcERms481JhPe7stgSEihqOISdgeo+NHVGhO6WMoD23OtlvUSnYa5ST A04yZ7ngd9cLOb2rQ+jg==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.89 #1 (Red Hat Linux)) id 1ej3V8-0002Xz-Vx; Tue, 06 Feb 2018 13:41:34 +0000 Received: from mail2-relais-roc.national.inria.fr ([192.134.164.83]) by bombadil.infradead.org with esmtps (Exim 4.89 #1 (Red Hat Linux)) id 1ej3V0-0002Jo-5e for linux-arm-kernel@lists.infradead.org; Tue, 06 Feb 2018 13:41:33 +0000 X-IronPort-AV: E=Sophos;i="5.46,468,1511823600"; d="scan'208";a="312276165" Received: from vaio-julia.rsr.lip6.fr ([132.227.76.33]) by mail2-relais-roc.national.inria.fr with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 06 Feb 2018 14:40:39 +0100 Date: Tue, 6 Feb 2018 14:40:35 +0100 (CET) From: Julia Lawall X-X-Sender: jll@hadrien To: Dan Carpenter Subject: Re: [PATCH 0/4] tree-wide: fix comparison to bitshift when dealing with a mask In-Reply-To: <20180206132335.luut6em3kut7f7ej@mwanda> Message-ID: References: <20180205201002.23621-1-wsa+renesas@sang-engineering.com> <20180206131044.oso33fvv553trrd7@mwanda> <20180206132335.luut6em3kut7f7ej@mwanda> User-Agent: Alpine 2.20 (DEB 67 2015-01-07) MIME-Version: 1.0 X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: linux-samsung-soc@vger.kernel.org, netdev@vger.kernel.org, linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, linux-renesas-soc@vger.kernel.org, Wolfram Sang , linux-arm-kernel@lists.infradead.org, linux-media@vger.kernel.org Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org X-Virus-Scanned: ClamAV using ClamSMTP On Tue, 6 Feb 2018, Dan Carpenter wrote: > On Tue, Feb 06, 2018 at 02:15:51PM +0100, Julia Lawall wrote: > > > > > > On Tue, 6 Feb 2018, Dan Carpenter wrote: > > > > > On Mon, Feb 05, 2018 at 09:09:57PM +0100, Wolfram Sang wrote: > > > > In one Renesas driver, I found a typo which turned an intended bit shift ('<<') > > > > into a comparison ('<'). Because this is a subtle issue, I looked tree wide for > > > > similar patterns. This small patch series is the outcome. > > > > > > > > Buildbot and checkpatch are happy. Only compile-tested. To be applied > > > > individually per sub-system, I think. I'd think only the net: amd: patch needs > > > > to be conisdered for stable, but I leave this to people who actually know this > > > > driver. > > > > > > > > CCing Dan. Maybe he has an idea how to add a test to smatch? In my setup, only > > > > cppcheck reported a 'coding style' issue with a low prio. > > > > > > > > > > Most of these are inside macros so it makes it complicated for Smatch > > > to warn about them. It might be easier in Coccinelle. Julia the bugs > > > look like this: > > > > > > - reissue_mask |= 0xffff < 4; > > > + reissue_mask |= 0xffff << 4; > > > > Thanks. I'll take a look. Do you have an example of the macro issue > > handy? > > > > It's the same: > > #define EXYNOS_CIIMGEFF_PAT_CBCR_MASK ((0xff < 13) | (0xff < 0)) > > Smatch only sees the outside of the macro (where it is used in the code) > and the pre-processed code. I wrote the following rule: @@ constant int x,y; identifier i; type T; @@ ( i < x | x < i | (T)i < x | x < (T)i | * x < y ) and got the results below. I can make a version for the kernel shortly. julia diff -u -p /run/shm/linux-next/drivers/gpu/drm/exynos/regs-fimc.h /tmp/nothing/drivers/gpu/drm/exynos/regs-fimc.h --- /run/shm/linux-next/drivers/gpu/drm/exynos/regs-fimc.h +++ /tmp/nothing/drivers/gpu/drm/exynos/regs-fimc.h @@ -569,7 +569,6 @@ #define EXYNOS_CIIMGEFF_FIN_EMBOSSING (4 << 26) #define EXYNOS_CIIMGEFF_FIN_SILHOUETTE (5 << 26) #define EXYNOS_CIIMGEFF_FIN_MASK (7 << 26) -#define EXYNOS_CIIMGEFF_PAT_CBCR_MASK ((0xff < 13) | (0xff < 0)) /* Real input DMA size register */ #define EXYNOS_CIREAL_ISIZE_AUTOLOAD_ENABLE (1 << 31) diff -u -p /run/shm/linux-next/drivers/net/ethernet/amd/xgbe/xgbe-drv.c /tmp/nothing/drivers/net/ethernet/amd/xgbe/xgbe-drv.c --- /run/shm/linux-next/drivers/net/ethernet/amd/xgbe/xgbe-drv.c +++ /tmp/nothing/drivers/net/ethernet/amd/xgbe/xgbe-drv.c @@ -595,7 +595,6 @@ isr_done: reissue_mask = 1 << 0; if (!pdata->per_channel_irq) - reissue_mask |= 0xffff < 4; XP_IOWRITE(pdata, XP_INT_REISSUE_EN, reissue_mask); } diff -u -p /run/shm/linux-next/drivers/video/fbdev/mxsfb.c /tmp/nothing/drivers/video/fbdev/mxsfb.c --- /run/shm/linux-next/drivers/video/fbdev/mxsfb.c +++ /tmp/nothing/drivers/video/fbdev/mxsfb.c @@ -133,8 +133,6 @@ #define VDCTRL4_SYNC_SIGNALS_ON (1 << 18) #define SET_DOTCLK_H_VALID_DATA_CNT(x) ((x) & 0x3ffff) -#define DEBUG0_HSYNC (1 < 26) -#define DEBUG0_VSYNC (1 < 25) #define MIN_XRES 120 #define MIN_YRES 120 diff -u -p /run/shm/linux-next/include/drm/drm_scdc_helper.h /tmp/nothing/include/drm/drm_scdc_helper.h --- /run/shm/linux-next/include/drm/drm_scdc_helper.h +++ /tmp/nothing/include/drm/drm_scdc_helper.h @@ -50,9 +50,6 @@ #define SCDC_READ_REQUEST_ENABLE (1 << 0) #define SCDC_STATUS_FLAGS_0 0x40 -#define SCDC_CH2_LOCK (1 < 3) -#define SCDC_CH1_LOCK (1 < 2) -#define SCDC_CH0_LOCK (1 < 1) #define SCDC_CH_LOCK_MASK (SCDC_CH2_LOCK | SCDC_CH1_LOCK | SCDC_CH0_LOCK) #define SCDC_CLOCK_DETECT (1 << 0) diff -u -p /run/shm/linux-next/arch/um/drivers/vector_user.h /tmp/nothing/arch/um/drivers/vector_user.h --- /run/shm/linux-next/arch/um/drivers/vector_user.h +++ /tmp/nothing/arch/um/drivers/vector_user.h @@ -61,8 +61,6 @@ struct vector_fds { }; #define VECTOR_READ 1 -#define VECTOR_WRITE (1 < 1) -#define VECTOR_HEADERS (1 < 2) extern struct arglist *uml_parse_vector_ifspec(char *arg); diff -u -p /run/shm/linux-next/drivers/gpu/drm/mxsfb/mxsfb_regs.h /tmp/nothing/drivers/gpu/drm/mxsfb/mxsfb_regs.h --- /run/shm/linux-next/drivers/gpu/drm/mxsfb/mxsfb_regs.h +++ /tmp/nothing/drivers/gpu/drm/mxsfb/mxsfb_regs.h @@ -91,8 +91,6 @@ #define VDCTRL4_SYNC_SIGNALS_ON (1 << 18) #define SET_DOTCLK_H_VALID_DATA_CNT(x) ((x) & 0x3ffff) -#define DEBUG0_HSYNC (1 < 26) -#define DEBUG0_VSYNC (1 < 25) #define MXSFB_MIN_XRES 120 #define MXSFB_MIN_YRES 120 diff -u -p /run/shm/linux-next/drivers/media/platform/vsp1/vsp1_regs.h /tmp/nothing/drivers/media/platform/vsp1/vsp1_regs.h --- /run/shm/linux-next/drivers/media/platform/vsp1/vsp1_regs.h +++ /tmp/nothing/drivers/media/platform/vsp1/vsp1_regs.h @@ -225,7 +225,6 @@ #define VI6_RPF_MULT_ALPHA_P_MMD_RATIO (1 << 8) #define VI6_RPF_MULT_ALPHA_P_MMD_IMAGE (2 << 8) #define VI6_RPF_MULT_ALPHA_P_MMD_BOTH (3 << 8) -#define VI6_RPF_MULT_ALPHA_RATIO_MASK (0xff < 0) #define VI6_RPF_MULT_ALPHA_RATIO_SHIFT 0 /* ----------------------------------------------------------------------------- diff -u -p /run/shm/linux-next/drivers/media/dvb-frontends/stb0899_reg.h /tmp/nothing/drivers/media/dvb-frontends/stb0899_reg.h --- /run/shm/linux-next/drivers/media/dvb-frontends/stb0899_reg.h +++ /tmp/nothing/drivers/media/dvb-frontends/stb0899_reg.h @@ -374,22 +374,18 @@ #define STB0899_OFF0_IF_AGC_GAIN 0xf30c #define STB0899_BASE_IF_AGC_GAIN 0x00000000 -#define STB0899_IF_AGC_GAIN (0x3fff < 0) #define STB0899_OFFST_IF_AGC_GAIN 0 #define STB0899_WIDTH_IF_AGC_GAIN 14 #define STB0899_OFF0_BB_AGC_GAIN 0xf310 #define STB0899_BASE_BB_AGC_GAIN 0x00000000 -#define STB0899_BB_AGC_GAIN (0x3fff < 0) #define STB0899_OFFST_BB_AGC_GAIN 0 #define STB0899_WIDTH_BB_AGC_GAIN 14 #define STB0899_OFF0_DC_OFFSET 0xf314 #define STB0899_BASE_DC_OFFSET 0x00000000 -#define STB0899_I (0xff < 8) #define STB0899_OFFST_I 8 #define STB0899_WIDTH_I 8 -#define STB0899_Q (0xff < 0) #define STB0899_OFFST_Q 8 #define STB0899_WIDTH_Q 8