IS_ERR_VALUE misuses

IS_ERR_VALUE misuses

[Linus Torvalds]

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 of 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")

               Linus
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

IS_ERR_VALUE misuses

[Linus Torvalds]

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 of 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")

               Linus

Re: IS_ERR_VALUE misuses

[Dan Williams]

On Fri, May 27, 2016 at 1:15 PM, Linus Torvalds
<torvalds@linux-foundation.org> 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 of 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?

diff --git a/drivers/nvdimm/claim.c b/drivers/nvdimm/claim.c
index 8b2e3c4fb0ad..9997ff94a132 100644
--- a/drivers/nvdimm/claim.c
+++ b/drivers/nvdimm/claim.c
@@ -266,9 +266,8 @@ int devm_nsio_enable(struct device *dev, struct
nd_namespace_io *nsio)

        nsio->addr = devm_memremap(dev, res->start, resource_size(res),
                        ARCH_MEMREMAP_PMEM);
-       if (IS_ERR(nsio->addr))
-               return PTR_ERR(nsio->addr);
-       return 0;
+
+       return PTR_ERR_OR_ZERO(nsio->addr);
 }
 EXPORT_SYMBOL_GPL(devm_nsio_enable);

diff --git a/include/linux/err.h b/include/linux/err.h
index 56762ab41713..1e3558845e4c 100644
--- a/include/linux/err.h
+++ b/include/linux/err.h
@@ -18,7 +18,7 @@

 #ifndef __ASSEMBLY__

-#define IS_ERR_VALUE(x) unlikely((x) >= (unsigned long)-MAX_ERRNO)
+#define IS_ERR_VALUE(x) unlikely((unsigned long)(void *)(x) >=
(unsigned long)-MAX_ERRNO)

 static inline void * __must_check ERR_PTR(long error)
 {

$ git grep -n IS_ERR drivers/nvdimm/
drivers/nvdimm/blk.c:317:       if (IS_ERR(ndns))
drivers/nvdimm/btt.c:209:       if (IS_ERR_OR_NULL(d))
drivers/nvdimm/btt.c:241:       if (IS_ERR_OR_NULL(btt->debugfs_dir))
drivers/nvdimm/btt.c:1424:      if (IS_ERR_OR_NULL(debugfs_root))
drivers/nvdimm/bus.c:398:       if (IS_ERR(dev)) {
drivers/nvdimm/bus.c:848:       if (IS_ERR(nd_class)) {
drivers/nvdimm/pmem.c:223:              if (IS_ERR(altmap))
drivers/nvdimm/pmem.c:277:      if (IS_ERR(addr))
drivers/nvdimm/pmem.c:318:      if (IS_ERR(ndns))
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Re: IS_ERR_VALUE misuses

[Dan Williams]

On Fri, May 27, 2016 at 1:15 PM, Linus Torvalds
<torvalds@linux-foundation.org> 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 of 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?

diff --git a/drivers/nvdimm/claim.c b/drivers/nvdimm/claim.c
index 8b2e3c4fb0ad..9997ff94a132 100644
--- a/drivers/nvdimm/claim.c
+++ b/drivers/nvdimm/claim.c
@@ -266,9 +266,8 @@ int devm_nsio_enable(struct device *dev, struct
nd_namespace_io *nsio)

        nsio->addr = devm_memremap(dev, res->start, resource_size(res),
                        ARCH_MEMREMAP_PMEM);
-       if (IS_ERR(nsio->addr))
-               return PTR_ERR(nsio->addr);
-       return 0;
+
+       return PTR_ERR_OR_ZERO(nsio->addr);
 }
 EXPORT_SYMBOL_GPL(devm_nsio_enable);

diff --git a/include/linux/err.h b/include/linux/err.h
index 56762ab41713..1e3558845e4c 100644
--- a/include/linux/err.h
+++ b/include/linux/err.h
@@ -18,7 +18,7 @@

 #ifndef __ASSEMBLY__

-#define IS_ERR_VALUE(x) unlikely((x) >= (unsigned long)-MAX_ERRNO)
+#define IS_ERR_VALUE(x) unlikely((unsigned long)(void *)(x) >=
(unsigned long)-MAX_ERRNO)

 static inline void * __must_check ERR_PTR(long error)
 {

$ git grep -n IS_ERR drivers/nvdimm/
drivers/nvdimm/blk.c:317:       if (IS_ERR(ndns))
drivers/nvdimm/btt.c:209:       if (IS_ERR_OR_NULL(d))
drivers/nvdimm/btt.c:241:       if (IS_ERR_OR_NULL(btt->debugfs_dir))
drivers/nvdimm/btt.c:1424:      if (IS_ERR_OR_NULL(debugfs_root))
drivers/nvdimm/bus.c:398:       if (IS_ERR(dev)) {
drivers/nvdimm/bus.c:848:       if (IS_ERR(nd_class)) {
drivers/nvdimm/pmem.c:223:              if (IS_ERR(altmap))
drivers/nvdimm/pmem.c:277:      if (IS_ERR(addr))
drivers/nvdimm/pmem.c:318:      if (IS_ERR(ndns))

Re: IS_ERR_VALUE misuses

[Dan Williams]

[ Adding Srinivas ]

On Fri, May 27, 2016 at 1:41 PM, Dan Williams <dan.j.williams@intel.com> wrote:
> On Fri, May 27, 2016 at 1:15 PM, Linus Torvalds
> <torvalds@linux-foundation.org> 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]
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Re: IS_ERR_VALUE misuses

[Dan Williams]

[ Adding Srinivas ]

On Fri, May 27, 2016 at 1:41 PM, Dan Williams <dan.j.williams@intel.com> wrote:
> On Fri, May 27, 2016 at 1:15 PM, Linus Torvalds
> <torvalds@linux-foundation.org> 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@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: IS_ERR_VALUE misuses

[Rafael J. Wysocki]

On Friday, May 27, 2016 01:15:12 PM Linus Torvalds 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 of 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.

Under drivers/acpi/ I could only find IS_ERR_VALUE() in acpi_dbg.c, but
those instances should be removed by the Arnd's patch if I'm not mistaken.

Thanks,
Rafael

_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Re: IS_ERR_VALUE misuses

[Rafael J. Wysocki]

On Friday, May 27, 2016 01:15:12 PM Linus Torvalds 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 of 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.

Under drivers/acpi/ I could only find IS_ERR_VALUE() in acpi_dbg.c, but
those instances should be removed by the Arnd's patch if I'm not mistaken.

Thanks,
Rafael

Re: IS_ERR_VALUE misuses

[Linus Torvalds]

On Sat, May 28, 2016 at 5:13 AM, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
>
> Under drivers/acpi/ I could only find IS_ERR_VALUE() in acpi_dbg.c, but
> those instances should be removed by the Arnd's patch if I'm not mistaken.

Yes, I only saw that Arnd had a patch for this and asked him to just
send it to me after I had already looked at this and sent the email
out..

The acpi/nvdimm misuses are gone now, although other suspect uses of
that macro do remain..

           Linus
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Re: IS_ERR_VALUE misuses

[Linus Torvalds]

On Sat, May 28, 2016 at 5:13 AM, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
>
> Under drivers/acpi/ I could only find IS_ERR_VALUE() in acpi_dbg.c, but
> those instances should be removed by the Arnd's patch if I'm not mistaken.

Yes, I only saw that Arnd had a patch for this and asked him to just
send it to me after I had already looked at this and sent the email
out..

The acpi/nvdimm misuses are gone now, although other suspect uses of
that macro do remain..

           Linus

RE: IS_ERR_VALUE misuses

[Zheng, Lv]

Hi, Rafael, Linus, Arnd

The instances in drivers/acpi/acpi_dbg.c were made by me.
Sorry for the issue and thanks for fixing it for me.

Best regards
-Lv

> From: linus971@gmail.com [mailto:linus971@gmail.com] On Behalf Of
> Linus Torvalds
> Subject: Re: IS_ERR_VALUE misuses
> 
> On Sat, May 28, 2016 at 5:13 AM, Rafael J. Wysocki <rjw@rjwysocki.net>
> wrote:
> >
> > Under drivers/acpi/ I could only find IS_ERR_VALUE() in acpi_dbg.c, but
> > those instances should be removed by the Arnd's patch if I'm not
> mistaken.
> 
> Yes, I only saw that Arnd had a patch for this and asked him to just
> send it to me after I had already looked at this and sent the email
> out..
> 
> The acpi/nvdimm misuses are gone now, although other suspect uses of
> that macro do remain..
> 
>            Linus
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

RE: IS_ERR_VALUE misuses

[Zheng, Lv]

Hi, Rafael, Linus, Arnd

The instances in drivers/acpi/acpi_dbg.c were made by me.
Sorry for the issue and thanks for fixing it for me.

Best regards
-Lv

> From: linus971@gmail.com [mailto:linus971@gmail.com] On Behalf Of
> Linus Torvalds
> Subject: Re: IS_ERR_VALUE misuses
> 
> On Sat, May 28, 2016 at 5:13 AM, Rafael J. Wysocki <rjw@rjwysocki.net>
> wrote:
> >
> > Under drivers/acpi/ I could only find IS_ERR_VALUE() in acpi_dbg.c, but
> > those instances should be removed by the Arnd's patch if I'm not
> mistaken.
> 
> Yes, I only saw that Arnd had a patch for this and asked him to just
> send it to me after I had already looked at this and sent the email
> out..
> 
> The acpi/nvdimm misuses are gone now, although other suspect uses of
> that macro do remain..
> 
>            Linus