Re: [PATCH v3] err.h: allow IS_ERR_VALUE to handle properly more types

From: Arnd Bergmann
Date: Mon Feb 08 2016 - 07:01:53 EST


On Monday 08 February 2016 09:45:55 Andrzej Hajda wrote:
> On 02/05/2016 11:52 AM, Arnd Bergmann wrote:
> > On Thursday 04 February 2016 10:59:31 Andrew Morton wrote:

> My version produces shortest code, Arnd's is the same as the old one.
> On the other side Rasmus proposition seems to be the most straightforward
> to me. Anyway I am not sure if the code length is the most important here.
>
> By the way .data segment size grows almost 4 times between gcc 4.4 and
> 4.8 :)
> Also numbers for arm64 looks interesting.
>
> Just for the record below all proposed implementations:
> #define IS_ERR_VALUE_old(x) unlikely((x) >= (unsigned long)-MAX_ERRNO)
> #define IS_ERR_VALUE_andrzej(x) ((typeof(x))(-1) <= 0 \
> ? unlikely((x) <= -1) \
> : unlikely((x) >= (typeof(x))-MAX_ERRNO))
> #define IS_ERR_VALUE_arnd(x) (unlikely((unsigned long long)(x) >=
> (unsigned long long)(typeof(x))-MAX_ERRNO))
> #define IS_ERR_VALUE_rasmus(x) ({\
> typeof(x) _x = (x);\
> unlikely(_x >= (typeof(x))-MAX_ERRNO && _x <= (typeof(x))-1);\
> })
>
> >
> > Andrzej's version is a little shorter on ARM because in case of signed numbers
> > it only checks for negative values, rather than checking for values in the
> > [-MAX_ERRNO..-1] range. I think the original behavior is more logical
> > in this case, and my version restores it.
>
> As I looked at the usage of the macro in the kernel I have not found any
> code
> which could benefit from the original behavior, except some buggy code in
> staging which have already pending fix[1].
> But maybe it would be better to use IS_ERR_VALUE to always check if err
> is in
> range [-MAX_ERRNO..-1] and just use simple 'err < 0' in typical case of
> signed types.

If we do that, should we also make it illegal to use an invalid type
for IS_ERR()? At least that could also catch any use of 'char' and 'unsigned
char' that are still broken.

diff --git a/include/linux/err.h b/include/linux/err.h
index 039b80bb56ae..6700ad40f73f 100644
--- a/include/linux/err.h
+++ b/include/linux/err.h
@@ -18,7 +18,7 @@

#ifndef __ASSEMBLY__

-#define IS_ERR_VALUE(x) (unlikely((unsigned long long)(x) >= (unsigned long long)(typeof(x))-MAX_ERRNO))
+#define IS_ERR_VALUE(x) ({typeof(x) max_errno = -MAX_ERRNO; compiletime_assert(max_errno > 0, "IS_ERR_VALUE takes an unsigned argument"); (x >= max_errno); })

static inline void * __must_check ERR_PTR(long error)
{


Unfortunately, we get a significant number of errors if we do that, I guess the
list is what you had before

Arnd

/git/arm-soc/drivers/irqchip/irq-hip04.c: In function 'hip04_of_init':
/git/arm-soc/drivers/irqchip/irq-hip04.c:405:203: error: call to '__compiletime_assert_405' declared with attribute error: IS_ERR_VALUE takes an unsigned argument
make[3]: *** [drivers/irqchip/irq-hip04.o] Error 1
/git/arm-soc/drivers/irqchip/irq-gic.c: In function '__gic_init_bases':
/git/arm-soc/drivers/irqchip/irq-gic.c:1106:205: error: call to '__compiletime_assert_1106' declared with attribute error: IS_ERR_VALUE takes an unsigned argument
make[3]: *** [drivers/irqchip/irq-gic.o] Error 1
make[3]: Target `__build' not remade because of errors.
make[2]: *** [drivers/irqchip] Error 2
/git/arm-soc/kernel/pid.c: In function 'alloc_pid':
/git/arm-soc/kernel/pid.c:314:198: error: call to '__compiletime_assert_314' declared with attribute error: IS_ERR_VALUE takes an unsigned argument
/git/arm-soc/kernel/pid.c:314:198: error: call to '__compiletime_assert_314' declared with attribute error: IS_ERR_VALUE takes an unsigned argument
make[3]: *** [kernel/pid.o] Error 1
In function 'arm_smmu_init_domain_context',
inlined from 'arm_smmu_attach_dev' at /git/arm-soc/drivers/iommu/arm-smmu.c:1138:6:
/git/arm-soc/drivers/iommu/arm-smmu.c:877:198: error: call to '__compiletime_assert_877' declared with attribute error: IS_ERR_VALUE takes an unsigned argument
/git/arm-soc/drivers/iommu/arm-smmu.c:916:198: error: call to '__compiletime_assert_916' declared with attribute error: IS_ERR_VALUE takes an unsigned argument
/git/arm-soc/drivers/iommu/arm-smmu.c: In function 'arm_smmu_attach_dev':
/git/arm-soc/drivers/iommu/arm-smmu.c:1139:199: error: call to '__compiletime_assert_1139' declared with attribute error: IS_ERR_VALUE takes an unsigned argument
make[3]: *** [drivers/iommu/arm-smmu.o] Error 1
/git/arm-soc/drivers/dma/sun4i-dma.c: In function 'generate_ndma_promise':
/git/arm-soc/drivers/dma/sun4i-dma.c:464:198: error: call to '__compiletime_assert_464' declared with attribute error: IS_ERR_VALUE takes an unsigned argument
/git/arm-soc/drivers/dma/sun4i-dma.c: In function 'generate_ddma_promise':
/git/arm-soc/drivers/dma/sun4i-dma.c:521:198: error: call to '__compiletime_assert_521' declared with attribute error: IS_ERR_VALUE takes an unsigned argument
make[3]: *** [drivers/dma/sun4i-dma.o] Error 1
make[3]: Target `__build' not remade because of errors.
make[2]: *** [drivers/iommu] Error 2
In function 'pl011_probe_dt_alias',
inlined from 'pl011_setup_port' at /git/arm-soc/drivers/tty/serial/amba-pl011.c:2414:8:
/git/arm-soc/drivers/tty/serial/amba-pl011.c:2360:199: error: call to '__compiletime_assert_2360' declared with attribute error: IS_ERR_VALUE takes an unsigned argument
make[4]: *** [drivers/tty/serial/amba-pl011.o] Error 1
/git/arm-soc/drivers/tty/serial/clps711x.c: In function 'uart_clps711x_probe':
/git/arm-soc/drivers/tty/serial/clps711x.c:475:204: error: call to '__compiletime_assert_475' declared with attribute error: IS_ERR_VALUE takes an unsigned argument
make[4]: *** [drivers/tty/serial/clps711x.o] Error 1
/git/arm-soc/drivers/tty/serial/8250/8250_ingenic.c:337:252: error: data definition has no type or storage class [-Werror]
/git/arm-soc/drivers/tty/serial/8250/8250_ingenic.c:337:252: error: type defaults to 'int' in declaration of 'device_initcall' [-Werror=implicit-int]
/git/arm-soc/drivers/tty/serial/8250/8250_ingenic.c:337:1: error: parameter names (without types) in function declaration [-Werror]
builtin_platform_driver(ingenic_uart_platform_driver);
^
/git/arm-soc/drivers/tty/serial/8250/8250_ingenic.c:337:122: error: 'ingenic_uart_platform_driver_init' defined but not used [-Werror=unused-function]
cc1: all warnings being treated as errors
make[5]: *** [drivers/tty/serial/8250/8250_ingenic.o] Error 1
make[5]: Target `__build' not remade because of errors.
make[4]: *** [drivers/tty/serial/8250] Error 2
make[3]: Target `__build' not remade because of errors.
make[2]: *** [drivers/dma] Error 2
/git/arm-soc/drivers/mfd/twl4030-irq.c: In function 'twl4030_init_irq':
/git/arm-soc/drivers/mfd/twl4030-irq.c:699:203: error: call to '__compiletime_assert_699' declared with attribute error: IS_ERR_VALUE takes an unsigned argument
make[3]: *** [drivers/mfd/twl4030-irq.o] Error 1
make[4]: Target `__build' not remade because of errors.
make[3]: *** [drivers/tty/serial] Error 2
/git/arm-soc/fs/afs/write.c: In function 'afs_file_write':
/git/arm-soc/fs/afs/write.c:646:201: error: call to '__compiletime_assert_646' declared with attribute error: IS_ERR_VALUE takes an unsigned argument
make[4]: *** [fs/afs/write.o] Error 1
make[4]: Target `__build' not remade because of errors.
make[3]: *** [fs/afs] Error 2
In function 'da8xx_fb_config_clk_divider',
inlined from 'da8xx_fb_calc_config_clk_divider' at /git/arm-soc/drivers/video/fbdev/da8xx-fb.c:769:9:
/git/arm-soc/drivers/video/fbdev/da8xx-fb.c:717:199: error: call to '__compiletime_assert_717' declared with attribute error: IS_ERR_VALUE takes an unsigned argument
In function 'lcd_init',
inlined from 'da8xxfb_set_par' at /git/arm-soc/drivers/video/fbdev/da8xx-fb.c:1284:6:
/git/arm-soc/drivers/video/fbdev/da8xx-fb.c:788:198: error: call to '__compiletime_assert_788' declared with attribute error: IS_ERR_VALUE takes an unsigned argument
make[4]: *** [drivers/video/fbdev/da8xx-fb.o] Error 1
make[3]: Target `__build' not remade because of errors.
make[2]: *** [drivers/tty] Error 2
make[4]: Target `__build' not remade because of errors.
make[3]: *** [drivers/video/fbdev] Error 2
make[3]: Target `__build' not remade because of errors.
make[2]: *** [drivers/video] Error 2
/git/arm-soc/drivers/cpufreq/omap-cpufreq.c: In function 'omap_target':
/git/arm-soc/drivers/cpufreq/omap-cpufreq.c:54:197: error: call to '__compiletime_assert_54' declared with attribute error: IS_ERR_VALUE takes an unsigned argument
make[3]: *** [drivers/cpufreq/omap-cpufreq.o] Error 1
In function 'mmc_select_hs200',
inlined from 'mmc_select_timing' at /git/arm-soc/drivers/mmc/core/mmc.c:1310:7,
inlined from 'mmc_init_card' at /git/arm-soc/drivers/mmc/core/mmc.c:1565:6:
/git/arm-soc/drivers/mmc/core/mmc.c:1271:200: error: call to '__compiletime_assert_1271' declared with attribute error: IS_ERR_VALUE takes an unsigned argument
/git/arm-soc/drivers/mmc/core/mmc.c: In function 'mmc_init_card':
/git/arm-soc/drivers/mmc/core/mmc.c:1580:201: error: call to '__compiletime_assert_1580' declared with attribute error: IS_ERR_VALUE takes an unsigned argument
make[4]: *** [drivers/mmc/core/mmc.o] Error 1
make[3]: Target `__build' not remade because of errors.
make[2]: *** [drivers/cpufreq] Error 2
/git/arm-soc/drivers/crypto/caam/ctrl.c: In function 'caam_get_era':
/git/arm-soc/drivers/crypto/caam/ctrl.c:405:201: error: call to '__compiletime_assert_405' declared with attribute error: IS_ERR_VALUE takes an unsigned argument
make[4]: *** [drivers/crypto/caam/ctrl.o] Error 1
make[4]: Target `__build' not remade because of errors.
make[3]: *** [drivers/mmc/core] Error 2
make[4]: Target `__build' not remade because of errors.
make[3]: *** [drivers/crypto/caam] Error 2
In file included from /git/arm-soc/include/linux/clk.h:15:0,
from /git/arm-soc/drivers/crypto/ux500/cryp/cryp_core.c:12:
/git/arm-soc/include/linux/err.h: In function 'IS_ERR':
/git/arm-soc/include/linux/err.h:35:215: error: call to '__compiletime_assert_35' declared with attribute error: IS_ERR_VALUE takes an unsigned argument
make[5]: *** [drivers/crypto/ux500/cryp/cryp_core.o] Error 1
make[5]: Target `__build' not remade because of errors.
make[4]: *** [drivers/crypto/ux500/cryp] Error 2
/git/arm-soc/drivers/mmc/host/sdhci.c: In function 'sdhci_do_get_cd':
/git/arm-soc/drivers/mmc/host/sdhci.c:1628:204: error: call to '__compiletime_assert_1628' declared with attribute error: IS_ERR_VALUE takes an unsigned argument
/git/arm-soc/drivers/mmc/host/sdhci.c: In function 'sdhci_add_host':
/git/arm-soc/drivers/mmc/host/sdhci.c:3117:222: error: call to '__compiletime_assert_3117' declared with attribute error: IS_ERR_VALUE takes an unsigned argument
make[4]: *** [drivers/mmc/host/sdhci.o] Error 1
In file included from /git/arm-soc/include/linux/clk.h:15:0,
from /git/arm-soc/drivers/crypto/ux500/hash/hash_core.c:16:
/git/arm-soc/include/linux/err.h: In function 'IS_ERR':
/git/arm-soc/include/linux/err.h:35:215: error: call to '__compiletime_assert_35' declared with attribute error: IS_ERR_VALUE takes an unsigned argument
make[5]: *** [drivers/crypto/ux500/hash/hash_core.o] Error 1
make[5]: Target `__build' not remade because of errors.
make[4]: *** [drivers/crypto/ux500/hash] Error 2
make[4]: Target `__build' not remade because of errors.
make[3]: *** [drivers/crypto/ux500] Error 2
make[3]: Target `__build' not remade because of errors.
make[2]: *** [drivers/crypto] Error 2
In file included from /git/arm-soc/drivers/gpu/host1x/hw/host1x01.c:27:0:
/git/arm-soc/drivers/gpu/host1x/hw/intr_hw.c: In function '_host1x_intr_init_host_sync':
/git/arm-soc/drivers/gpu/host1x/hw/intr_hw.c:88:197: error: call to '__compiletime_assert_88' declared with attribute error: IS_ERR_VALUE takes an unsigned argument
make[4]: *** [drivers/gpu/host1x/hw/host1x01.o] Error 1
In file included from /git/arm-soc/drivers/gpu/host1x/hw/host1x02.c:27:0:
/git/arm-soc/drivers/gpu/host1x/hw/intr_hw.c: In function '_host1x_intr_init_host_sync':
/git/arm-soc/drivers/gpu/host1x/hw/intr_hw.c:88:197: error: call to '__compiletime_assert_88' declared with attribute error: IS_ERR_VALUE takes an unsigned argument
make[4]: *** [drivers/gpu/host1x/hw/host1x02.o] Error 1
In file included from /git/arm-soc/drivers/gpu/host1x/hw/host1x04.c:27:0:
/git/arm-soc/drivers/gpu/host1x/hw/intr_hw.c: In function '_host1x_intr_init_host_sync':
/git/arm-soc/drivers/gpu/host1x/hw/intr_hw.c:88:197: error: call to '__compiletime_assert_88' declared with attribute error: IS_ERR_VALUE takes an unsigned argument
make[4]: *** [drivers/gpu/host1x/hw/host1x04.o] Error 1
In file included from /git/arm-soc/drivers/gpu/host1x/hw/host1x05.c:27:0:
/git/arm-soc/drivers/gpu/host1x/hw/intr_hw.c: In function '_host1x_intr_init_host_sync':
/git/arm-soc/drivers/gpu/host1x/hw/intr_hw.c:88:197: error: call to '__compiletime_assert_88' declared with attribute error: IS_ERR_VALUE takes an unsigned argument
make[4]: *** [drivers/gpu/host1x/hw/host1x05.o] Error 1
make[4]: Target `__build' not remade because of errors.
make[3]: *** [drivers/gpu/host1x] Error 2
/git/arm-soc/drivers/mmc/host/dw_mmc.c: In function 'dw_mci_get_ro':
/git/arm-soc/drivers/mmc/host/dw_mmc.c:1434:204: error: call to '__compiletime_assert_1434' declared with attribute error: IS_ERR_VALUE takes an unsigned argument
/git/arm-soc/drivers/mmc/host/dw_mmc.c: In function 'dw_mci_get_cd':
/git/arm-soc/drivers/mmc/host/dw_mmc.c:1457:209: error: call to '__compiletime_assert_1457' declared with attribute error: IS_ERR_VALUE takes an unsigned argument
/git/arm-soc/drivers/mmc/host/dw_mmc.c: In function 'dw_mci_enable_cd':
/git/arm-soc/drivers/mmc/host/dw_mmc.c:2930:223: error: call to '__compiletime_assert_2930' declared with attribute error: IS_ERR_VALUE takes an unsigned argument
make[4]: *** [drivers/mmc/host/dw_mmc.o] Error 1
make[3]: Target `__build' not remade because of errors.
make[2]: *** [drivers/mfd] Error 2
/git/arm-soc/fs/gfs2/dir.c: In function 'get_first_leaf':
/git/arm-soc/fs/gfs2/dir.c:801:201: error: call to '__compiletime_assert_801' declared with attribute error: IS_ERR_VALUE takes an unsigned argument
/git/arm-soc/fs/gfs2/dir.c: In function 'dir_split_leaf':
/git/arm-soc/fs/gfs2/dir.c:1017:201: error: call to '__compiletime_assert_1017' declared with attribute error: IS_ERR_VALUE takes an unsigned argument
make[4]: *** [fs/gfs2/dir.o] Error 1
In function 'sdhci_esdhc_imx_probe_dt',
inlined from 'sdhci_esdhc_imx_probe' at /git/arm-soc/drivers/mmc/host/sdhci-esdhc-imx.c:1213:7:
/git/arm-soc/drivers/mmc/host/sdhci-esdhc-imx.c:1014:223: error: call to '__compiletime_assert_1014' declared with attribute error: IS_ERR_VALUE takes an unsigned argument
make[4]: *** [drivers/mmc/host/sdhci-esdhc-imx.o] Error 1
make[4]: Target `__build' not remade because of errors.
make[3]: *** [drivers/mmc/host] Error 2
make[3]: Target `__build' not remade because of errors.
make[2]: *** [drivers/mmc] Error 2
In function 'highbank_set_em_messages',
inlined from 'ahci_highbank_probe' at /git/arm-soc/drivers/ata/sata_highbank.c:537:2:
/git/arm-soc/drivers/ata/sata_highbank.c:200:199: error: call to '__compiletime_assert_200' declared with attribute error: IS_ERR_VALUE takes an unsigned argument
make[3]: *** [drivers/ata/sata_highbank.o] Error 1
make[4]: Target `__build' not remade because of errors.
make[3]: *** [fs/gfs2] Error 2
make[3]: Target `__build' not remade because of errors.
make[2]: *** [kernel] Error 2
make[3]: Target `__build' not remade because of errors.
make[2]: *** [drivers/ata] Error 2
/git/arm-soc/drivers/nvmem/core.c: In function 'bin_attr_nvmem_write':
/git/arm-soc/drivers/nvmem/core.c:105:197: error: call to '__compiletime_assert_105' declared with attribute error: IS_ERR_VALUE takes an unsigned argument
/git/arm-soc/drivers/nvmem/core.c: In function 'bin_attr_nvmem_read':
/git/arm-soc/drivers/nvmem/core.c:80:196: error: call to '__compiletime_assert_80' declared with attribute error: IS_ERR_VALUE takes an unsigned argument
/git/arm-soc/drivers/nvmem/core.c: In function '__nvmem_cell_read':
/git/arm-soc/drivers/nvmem/core.c:824:197: error: call to '__compiletime_assert_824' declared with attribute error: IS_ERR_VALUE takes an unsigned argument
/git/arm-soc/drivers/nvmem/core.c: In function 'nvmem_device_cell_read':
/git/arm-soc/drivers/nvmem/core.c:972:197: error: call to '__compiletime_assert_972' declared with attribute error: IS_ERR_VALUE takes an unsigned argument
/git/arm-soc/drivers/nvmem/core.c:976:197: error: call to '__compiletime_assert_976' declared with attribute error: IS_ERR_VALUE takes an unsigned argument
/git/arm-soc/drivers/nvmem/core.c: In function 'nvmem_device_read':
/git/arm-soc/drivers/nvmem/core.c:1031:198: error: call to '__compiletime_assert_1031' declared with attribute error: IS_ERR_VALUE takes an unsigned argument
/git/arm-soc/drivers/nvmem/core.c: In function 'nvmem_device_write':
/git/arm-soc/drivers/nvmem/core.c:1059:198: error: call to '__compiletime_assert_1059' declared with attribute error: IS_ERR_VALUE takes an unsigned argument
/git/arm-soc/drivers/nvmem/core.c: In function 'nvmem_cell_write':
/git/arm-soc/drivers/nvmem/core.c:944:197: error: call to '__compiletime_assert_944' declared with attribute error: IS_ERR_VALUE takes an unsigned argument
/git/arm-soc/drivers/nvmem/core.c: In function 'nvmem_device_cell_write':
/git/arm-soc/drivers/nvmem/core.c:1002:198: error: call to '__compiletime_assert_1002' declared with attribute error: IS_ERR_VALUE takes an unsigned argument
In function 'nvmem_add_cells',
inlined from 'nvmem_register' at /git/arm-soc/drivers/nvmem/core.c:365:3:
/git/arm-soc/drivers/nvmem/core.c:277:200: error: call to '__compiletime_assert_277' declared with attribute error: IS_ERR_VALUE takes an unsigned argument
/git/arm-soc/drivers/nvmem/core.c: In function 'nvmem_cell_read':
/git/arm-soc/drivers/nvmem/core.c:859:197: error: call to '__compiletime_assert_859' declared with attribute error: IS_ERR_VALUE takes an unsigned argument
make[3]: *** [drivers/nvmem/core.o] Error 1
make[3]: Target `__build' not remade because of errors.