From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751956AbcBKGqV (ORCPT ); Thu, 11 Feb 2016 01:46:21 -0500 Received: from mailout1.w1.samsung.com ([210.118.77.11]:50895 "EHLO mailout1.w1.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751169AbcBKGqS (ORCPT ); Thu, 11 Feb 2016 01:46:18 -0500 MIME-version: 1.0 Content-type: text/plain; charset=utf-8 X-AuditID: cbfec7f5-f79b16d000005389-8e-56bc2e36c610 Content-transfer-encoding: 8BIT Subject: Re: [PATCH] tty/serial: digicolor: Fix bad usage of IS_ERR_VALUE To: Guenter Roeck , Arnd Bergmann References: <1455030539-10798-1-git-send-email-linux@roeck-us.net> <4571887.rTPndCnv20@wuerfel> <56BAA27A.9050605@roeck-us.net> <2606061.l722KO95iI@wuerfel> <56BC022B.7030004@roeck-us.net> Cc: linux-arm-kernel@lists.infradead.org, Greg Kroah-Hartman , Baruch Siach , linux-kernel@vger.kernel.org, linux-serial@vger.kernel.org, Jiri Slaby , kernel-testers@vger.kernel.org From: Andrzej Hajda Message-id: <56BC2DF7.20803@samsung.com> Date: Thu, 11 Feb 2016 07:45:11 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.5.1 In-reply-to: <56BC022B.7030004@roeck-us.net> X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFnrFLMWRmVeSWpSXmKPExsVy+t/xq7pmenvCDD4+sLT4O+kYu8WKa29Y LJoXr2ezmLLhA5PFv7YeZotNj6+xWlzeNYfN4sziXnaLJwvPMDlwevz+NYnRY//cNewem5fU e+z83sDusX7LVRaPuQ23mDw+b5ILYI/isklJzcksSy3St0vgyuj6+JqxYJ5kxe1FE5kbGNeI dDFycEgImEh0/9fqYuQEMsUkLtxbz9bFyMUhJLCUUeJh5xcmkASvgKDEj8n3WEDqmQXkJY5c yoYw1SWmTMmFKH/OKDH7wi1mkHJhAU+JrfPPgbWKCLhKPJ2/nwWi6ACjxNVjt5lAHGaBv4wS ++4+ZAepYhPQlPi7+SYbyFReAQ2JhZurQEwWAVWJy708IBWiAhEShzu7wKo5BXQkWv8sYpzA KDALyXWzEK6bhXDdAkbmVYyiqaXJBcVJ6blGesWJucWleel6yfm5mxghMfB1B+PSY1aHGAU4 GJV4eAPqd4cJsSaWFVfmHmKU4GBWEuHd8RgoxJuSWFmVWpQfX1Sak1p8iFGag0VJnHfmrvch QgLpiSWp2ampBalFMFkmDk6pBkaX+c7vvG035NxfqzCff2LdurlzFYu3u1y16BSJfPiq511A QqJc0C3RDfl/Dypu61RZdmnN9/LQh2/7q/+EKk8s+nTY+KMN7+aZTSdnnj26TzBUNGayj7yI 7Yuq61dTH2i+8w/J28mUWDz3cof28k9ZgopCKckG8t82XXTferr+KT//t3tnnD8osRRnJBpq MRcVJwIA3mvg5H0CAAA= Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 02/11/2016 04:38 AM, Guenter Roeck wrote: > On 02/10/2016 07:21 AM, Arnd Bergmann wrote: >> On Tuesday 09 February 2016 18:37:46 Guenter Roeck wrote: >>> On 02/09/2016 07:26 AM, Arnd Bergmann wrote: >>>> On Tuesday 09 February 2016 07:08:59 Guenter Roeck wrote: >>>>> IS_ERR_VALUE() assumes that its parameter is an unsigned long. >>>>> It can not be used to check if an unsigned int reflects an error. >>>>> Doing so can result in the following build warning. >>>>> >>>>> drivers/tty/serial/digicolor-usart.c: In function ‘digicolor_uart_probe’: >>>>> include/linux/err.h:21:38: warning: >>>>> comparison is always false due to limited range of data type >>>>> drivers/tty/serial/digicolor-usart.c:485:6: note: >>>>> in expansion of macro ‘IS_ERR_VALUE’ >>>>> >>>>> If that warning is seen, an error return from platform_get_irq() is missed. >>>>> >>>>> >>>> The patch looks correct to me, but what compiler version and which kernel >>>> tree is it that triggered the warning? >>>> >>>> Andrzej Hajda just modified the definition of IS_ERR_VALUE(), and the >>>> changes are still under discussion, but I don't see that warning with >>>> any of the versions. >>>> >>> I see it with gcc 5.1 and 5.2 (and W=1). I did not see / notice Andrzej's patch. >>> >>> I agree that fixing the problem in IS_ERR_VALUE() is preferrable. >>> >>> >> Ah, W=1 explains it. We are still debating about the proper solution. Al Viro >> pointed out that most users of IS_ERR_VALUE() shouldn't be using it at all, >> so your patch is probably best here after all. >> > Yes, after looking into it some more I agree. Coccinelle should be able to handle > most of the conversions automatically. I actually tried to write a script, just > for the fun of it, but it misses some of the problem cases in patch mode. > Maybe I get it working tonight. > > Guenter > Maybe it can be helpful. The script which classifies all IS_ERR_VALUE usages as correct, signed, incorrect and unknown (cocci was not able to detect type). Adding patch mode should be quite simple. // Options: --all-includes --include include/uapi/linux/netfilter_ipv6/ip6_tables.h --include include/uapi/linux/netfilter_ipv4/ip_tables.h --include include/uapi/linux/netfilter_arp/arp_tables.h --include include/uapi/linux/netfilter/x_tables.h --include include/linux/mm_types.h virtual context virtual report @r@ typedef bool, u8, u16, u32, u64, s8, s16, s32, s64, __u64, dma_addr_t, Elf32_Addr, Elf64_Addr; {unsigned long, size_t} ev; {short, int, long, s16, s32, ssize_t} ew; {char, unsigned char, unsigned short, unsigned int, long long, unsigned long long, bool, u8, u16, u32, u64, s64, __u64, dma_addr_t, Elf32_Addr, Elf64_Addr} ei; expression eu; position p; @@ IS_ERR_VALUE(\(ev \| ew \| ei \| eu \))@p @script:python depends on report@ p << r.p = []; ev << r.ev = ""; ew << r.ew = ""; ei << r.ei = ""; eu << r.eu = ""; @@ t = "" if ev: t = "correct" if ew: t = "signed" if ei: t = "incorrect" if eu: t = "unknown" e = ev + ew + ei + eu; if t: msg = "WARNING: %s argument type in IS_ERR_VALUE(%s)" % (t, e) coccilib.report.print_report(p[0], msg)