[ Adding Srinivas ] On Fri, May 27, 2016 at 1:41 PM, Dan Williams <dan.j.williams@xxxxxxxxx> wrote: > On Fri, May 27, 2016 at 1:15 PM, Linus Torvalds > <torvalds@xxxxxxxxxxxxxxxxxxxx> wrote: >> This is just a heads-up: for some reason the acpi layer and nvdimm use >> the IS_ERR_VALUE() macro, and they use it incorrectly. >> >> To see warnings about it, change the macro from >> >> #define IS_ERR_VALUE(x) unlikely((x) >= (unsigned long)-MAX_ERRNO) >> >> to do a cast to a pointer and back (ie make the "(x)" part be >> "(unsigned long)(void *)(x)" instead, which then will cause warnings >> like >> >> warning: cast to pointer from integer of different size >> [-Wint-to-pointer-cast] >> >> when passed an "int" argument. >> >> The reason "int" arguments are wrong is that the macro really is >> designed to test the upper range of a pointer value. It happens to >> work for signed integers too, but looking at the users, pretty much >> none ofdrivers/nvmem them are right. The ACPI and nvdimm users are all about the >> perfectly standard "zero for success, negative error code for >> failure", and so using >> >> if (IS_ERROR_VALUE(rc)) >> return rc; >> >> is just plain garbage. The code generally should just do >> >> if (rc) >> return rc; >> >> which is simpler, smaller, and generates better code. >> >> This bug seems to have been so common in the power management code >> that we even have a coccinelle script for it. But for some reason >> several uses remain in acpi_debug.c and now there are cases in >> drivers/nvdimm/ too. >> >> There are random various crap cases like that elsewhere too, but acpi >> and nvdimm were just more dense with this bug than most other places. >> >> The bug isn't fatal - as mentioned, IS_ERROR_VALUE() _does_ actually >> work on the range of integers that are normal errors, but it's >> pointless and actively misleading, and it's not meant for that use. So >> it just adds complexity and worse code generation for no actual gain. >> >> I noticed this when I was looking at similar idiocy in fs/gfs2, where >> the code in question caused warnings (for other reasons, but the main >> reason was "code too complex for gcc to understand it") >> > > So, I recompiled with that change and didn't see any new warnings. > While "make coccicheck" did point out the following clean up, I did > not see where drivers/nvdimm/ passes anything but a pointer to IS_ERR, > what am I missing? Ah, it looks like this feedback is meant for drivers/nvmem/ not drivers/nvdimm/ drivers/nvmem/core.c: In function ‘bin_attr_nvmem_read’: drivers/nvmem/core.c:116:41: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast] drivers/nvmem/core.c: In function ‘bin_attr_nvmem_write’: drivers/nvmem/core.c:150:41: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast] drivers/nvmem/core.c: In function ‘nvmem_add_cells’: drivers/nvmem/core.c:369:42: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast] drivers/nvmem/core.c: In function ‘__nvmem_cell_read’: drivers/nvmem/core.c:966:41: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast] drivers/nvmem/core.c: In function ‘nvmem_cell_read’: drivers/nvmem/core.c:1001:41: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast] drivers/nvmem/core.c: In function ‘nvmem_cell_write’: drivers/nvmem/core.c:1086:41: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast] drivers/nvmem/core.c: In function ‘nvmem_device_cell_read’: drivers/nvmem/core.c:1114:41: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast] drivers/nvmem/core.c:1118:41: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast] drivers/nvmem/core.c: In function ‘nvmem_device_cell_write’: drivers/nvmem/core.c:1144:41: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast] drivers/nvmem/core.c: In function ‘nvmem_device_read’: drivers/nvmem/core.c:1173:41: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast] drivers/nvmem/core.c: In function ‘nvmem_device_write’: drivers/nvmem/core.c:1201:41: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast] -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
