From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752399AbcAONpk (ORCPT ); Fri, 15 Jan 2016 08:45:40 -0500 Received: from mailout4.w1.samsung.com ([210.118.77.14]:9803 "EHLO mailout4.w1.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752101AbcAONpi (ORCPT ); Fri, 15 Jan 2016 08:45:38 -0500 X-AuditID: cbfec7f4-f79026d00000418a-c0-5698f7ff26c8 Subject: Re: [PATCH] err.h: allow IS_ERR_VALUE to handle properly more types To: Andrew Morton , open list References: <1452178705-2200-1-git-send-email-a.hajda@samsung.com> Cc: Bartlomiej Zolnierkiewicz , Marek Szyprowski Newsgroups: gmane.linux.kernel From: Andrzej Hajda Message-id: <5698F7E5.8080505@samsung.com> Date: Fri, 15 Jan 2016 14:45:09 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.5.1 MIME-version: 1.0 In-reply-to: <1452178705-2200-1-git-send-email-a.hajda@samsung.com> Content-type: text/plain; charset=windows-1252 Content-transfer-encoding: 7bit X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFjrDLMWRmVeSWpSXmKPExsVy+t/xa7r/v88IM9g6RdNizvo1bBYbZ6xn tbi8aw6bxdojd9kdWDxOzPjN4tG3ZRWjx+dNcgHMUVw2Kak5mWWpRfp2CVwZE5+uYimYZ1Lx 4e1H9gbG3bpdjJwcEgImEs1n/jFB2GISF+6tZ+ti5OIQEljKKLHmSTsjSEJI4DmjRP+VShBb WMBHYvGNxywgtohApMT5SZNYIGqcJJ4ufsgMYjMLpEu8PXgErJdPQE5i4tZnYHE2AU2Jv5tv soHYvAJaElMuQdSwCKhKrP18HiwuKhAhcbizix2iRlDix+R7YPM5BZwltr9oAarnAJqvJ3H/ ohbEKnmJzWveMk9gFJyFpGMWQtUsJFULGJlXMYqmliYXFCel5xrqFSfmFpfmpesl5+duYoSE 8ZcdjIuPWR1iFOBgVOLh/cE5I0yINbGsuDL3EKMEB7OSCK/2K6AQb0piZVVqUX58UWlOavEh RmkOFiVx3rm73ocICaQnlqRmp6YWpBbBZJk4OKUaGCeUTFgXv83usFhm0bdplZrhK3/9eWqz sN3u+MvgHK7tShtbjW9YCspJytet/fpuaZ7E4/T87tzaFgeWqZ6Rcxnr9YIX3C64dfWidYTn tFSmraef7vHQ6Xy/L+9XxW0mg1OiQeH6cYZ7OR6eXsO91Hd56BTeq1Znz7W8KWCe+fi4brRt Jdvhq0osxRmJhlrMRcWJAAIi2j9fAgAA Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Andrew, Have you looked at this patch? Are there chances to merge it to next? Two more things I have found: - current version of IS_ERR_VALUE does not work with unsigned long long on 32bit arch - commit message should be fixed, - gcc issues warnings when -Wtype-limits option is enabled, it can be easily silenced by changing first operator '<' to '<=' in the macro. Regards Andrzej On 01/07/2016 03:58 PM, Andrzej Hajda wrote: > Current implementation of IS_ERR_VALUE works correctly only with > following types: > - unsigned long, unsigned long long, > - short, int, long. > Other types are handled incorrectly either on 32-bit either on 64-bit > either on both architectures. > The patch fixes it by comparing argument with MAX_ERRNO casted > to argument's type for unsigned types and comparing with zero for signed > types. As a result all integer types bigger than char are handled properly. > > Signed-off-by: Andrzej Hajda > --- > Hi, > > I have analyzed usage of IS_ERR_VALUE using coccinelle and in about 35 cases > it is used incorrectly, ie it can hide errors depending of 32/64 bit architecture. > Instead of fixing usage I propose to enhance the macro to cover more types. > And just for record: the macro is used 101 times with signed variables, I am not > sure if it should be preferred over simple comparison "ret < 0", but the new version > can do it as well. > > In my previous attempt I have added type checking to the macro, I am not sure which > approach is better[1]. Anyway the previous patch did not catch developers attention. > > [1]: http://permalink.gmane.org/gmane.linux.kernel/2111532 > > And below list of detected potential errors: > drivers/char/mem.c:698:45-46: WARNING: incorrect argument type in IS_ERR_VALUE(( unsigned long long ) offset) > drivers/media/platform/soc_camera/atmel-isi.c:1089:21-22: WARNING: incorrect argument type in IS_ERR_VALUE(irq) > drivers/net/ethernet/freescale/fs_enet/mac-scc.c:149:36-37: WARNING: incorrect argument type in IS_ERR_VALUE(fep -> ring_mem_addr) > drivers/net/ethernet/freescale/ucc_geth.c:2237:48-49: WARNING: incorrect argument type in IS_ERR_VALUE(ugeth -> tx_bd_ring_offset [ j ]) > drivers/net/ethernet/freescale/ucc_geth.c:2314:48-49: WARNING: incorrect argument type in IS_ERR_VALUE(ugeth -> rx_bd_ring_offset [ j ]) > drivers/net/ethernet/freescale/ucc_geth.c:2524:44-45: WARNING: incorrect argument type in IS_ERR_VALUE(ugeth -> tx_glbl_pram_offset) > drivers/net/ethernet/freescale/ucc_geth.c:2544:45-46: WARNING: incorrect argument type in IS_ERR_VALUE(ugeth -> thread_dat_tx_offset) > drivers/net/ethernet/freescale/ucc_geth.c:2571:46-47: WARNING: incorrect argument type in IS_ERR_VALUE(ugeth -> send_q_mem_reg_offset) > drivers/net/ethernet/freescale/ucc_geth.c:2612:42-43: WARNING: incorrect argument type in IS_ERR_VALUE(ugeth -> scheduler_offset) > drivers/net/ethernet/freescale/ucc_geth.c:2659:54-55: WARNING: incorrect argument type in IS_ERR_VALUE(ugeth -> tx_fw_statistics_pram_offset) > drivers/net/ethernet/freescale/ucc_geth.c:2696:44-45: WARNING: incorrect argument type in IS_ERR_VALUE(ugeth -> rx_glbl_pram_offset) > drivers/net/ethernet/freescale/ucc_geth.c:2715:45-46: WARNING: incorrect argument type in IS_ERR_VALUE(ugeth -> thread_dat_rx_offset) > drivers/net/ethernet/freescale/ucc_geth.c:2736:54-55: WARNING: incorrect argument type in IS_ERR_VALUE(ugeth -> rx_fw_statistics_pram_offset) > drivers/net/ethernet/freescale/ucc_geth.c:2756:53-54: WARNING: incorrect argument type in IS_ERR_VALUE(ugeth -> rx_irq_coalescing_tbl_offset) > drivers/net/ethernet/freescale/ucc_geth.c:2822:44-45: WARNING: incorrect argument type in IS_ERR_VALUE(ugeth -> rx_bd_qs_tbl_offset) > drivers/net/ethernet/freescale/ucc_geth.c:2908:47-48: WARNING: incorrect argument type in IS_ERR_VALUE(ugeth -> exf_glbl_param_offset) > drivers/net/ethernet/freescale/ucc_geth.c:292:36-37: WARNING: incorrect argument type in IS_ERR_VALUE(init_enet_offset) > drivers/net/ethernet/freescale/ucc_geth.c:3042:39-40: WARNING: incorrect argument type in IS_ERR_VALUE(init_enet_pram_offset) > drivers/soc/fsl/qe/ucc_fast.c:271:60-61: WARNING: incorrect argument type in IS_ERR_VALUE(uccf -> ucc_fast_tx_virtual_fifo_base_offset) > drivers/soc/fsl/qe/ucc_fast.c:284:60-61: WARNING: incorrect argument type in IS_ERR_VALUE(uccf -> ucc_fast_rx_virtual_fifo_base_offset) > drivers/soc/fsl/qe/ucc_slow.c:186:38-39: WARNING: incorrect argument type in IS_ERR_VALUE(uccs -> us_pram_offset) > drivers/soc/fsl/qe/ucc_slow.c:213:38-39: WARNING: incorrect argument type in IS_ERR_VALUE(uccs -> rx_base_offset) > drivers/soc/fsl/qe/ucc_slow.c:224:38-39: WARNING: incorrect argument type in IS_ERR_VALUE(uccs -> tx_base_offset) > drivers/tty/serial/clps711x.c:471:29-30: WARNING: incorrect argument type in IS_ERR_VALUE(s -> port . irq) > drivers/tty/serial/digicolor-usart.c:485:30-31: WARNING: incorrect argument type in IS_ERR_VALUE(dp -> port . irq) > drivers/usb/gadget/udc/fsl_qe_udc.c:2369:26-27: WARNING: incorrect argument type in IS_ERR_VALUE(tmp_addr) > drivers/video/fbdev/exynos/exynos_mipi_dsi.c:406:27-28: WARNING: incorrect argument type in IS_ERR_VALUE(dsim -> irq) > net/ipv4/netfilter/arp_tables.c:1427:39-40: WARNING: incorrect argument type in IS_ERR_VALUE(iter1 -> counters . pcnt) > net/ipv4/netfilter/arp_tables.c:530:34-35: WARNING: incorrect argument type in IS_ERR_VALUE(e -> counters . pcnt) > net/ipv4/netfilter/ip_tables.c:1614:34-35: WARNING: incorrect argument type in IS_ERR_VALUE(e -> counters . pcnt) > net/ipv4/netfilter/ip_tables.c:674:34-35: WARNING: incorrect argument type in IS_ERR_VALUE(e -> counters . pcnt) > net/ipv6/netfilter/ip6_tables.c:1624:34-35: WARNING: incorrect argument type in IS_ERR_VALUE(e -> counters . pcnt) > net/ipv6/netfilter/ip6_tables.c:687:34-35: WARNING: incorrect argument type in IS_ERR_VALUE(e -> counters . pcnt) > drivers/net/ethernet/freescale/fs_enet/mac-fcc.c:110:35-36: WARNING: unknown argument type in IS_ERR_VALUE(fpi -> dpram_offset) > > Regards > Andrzej > > include/linux/err.h | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/include/linux/err.h b/include/linux/err.h > index 56762ab..40eca39 100644 > --- a/include/linux/err.h > +++ b/include/linux/err.h > @@ -18,7 +18,9 @@ > > #ifndef __ASSEMBLY__ > > -#define IS_ERR_VALUE(x) unlikely((x) >= (unsigned long)-MAX_ERRNO) > +#define IS_ERR_VALUE(x) ((typeof(x))(-1) < 0 \ > + ? unlikely((x) < 0) \ > + : unlikely((x) >= (typeof(x))-MAX_ERRNO)) > > static inline void * __must_check ERR_PTR(long error) > { >