From: Colin Ian King <[email protected]>
The previous fix removed the equal to zero comparisons by the strcmps and
now the function always returns true. Fix this by adding in the missing
logical negation operators.
Detected by CoverityScan, CID#1452267 ("Constant expression result")
Fixes: b93ad9a067e1 ("staging: fsl-mc: be consistent when checking strcmp() return")
Signed-off-by: Colin Ian King <[email protected]>
---
drivers/staging/fsl-mc/bus/fsl-mc-allocator.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/fsl-mc/bus/fsl-mc-allocator.c b/drivers/staging/fsl-mc/bus/fsl-mc-allocator.c
index b37a6f48225f..8b5f2d0cdf06 100644
--- a/drivers/staging/fsl-mc/bus/fsl-mc-allocator.c
+++ b/drivers/staging/fsl-mc/bus/fsl-mc-allocator.c
@@ -16,9 +16,9 @@
static bool __must_check fsl_mc_is_allocatable(const char *obj_type)
{
- return strcmp(obj_type, "dpbp") ||
- strcmp(obj_type, "dpmcp") ||
- strcmp(obj_type, "dpcon");
+ return !strcmp(obj_type, "dpbp") ||
+ !strcmp(obj_type, "dpmcp") ||
+ !strcmp(obj_type, "dpcon");
}
/**
--
2.11.0
On Wed, Aug 16, 2017 at 12:44:51PM +0100, Colin King wrote:
> From: Colin Ian King <[email protected]>
>
> The previous fix removed the equal to zero comparisons by the strcmps and
> now the function always returns true. Fix this by adding in the missing
> logical negation operators.
>
> Detected by CoverityScan, CID#1452267 ("Constant expression result")
>
> Fixes: b93ad9a067e1 ("staging: fsl-mc: be consistent when checking strcmp() return")
Ugh... I did review the original patch at all. Sorry. It's better to
use "== 0" because it's idiomatic.
strcmp(foo, bar) == 0 means foo == bar
strcmp(foo, bar) != 0 means foo != bar
strcmp(foo, bar) < 0 means foo < bar alphabetically.
It's way more readable. strcmp() bugs are fairly common when people
don't use == 0 and != 0. There are other places where != 0 hurts
readability, such as checking for errors:
if (frob() != 0) {
In this case, frob() is returning negative error codes, but it's not
really returning the number zero, it's returning "success". So it
should be:
ret = frob();
if (ret) {
Comparing against NULL really doesn't add anything either. But if
you're talking about the number zero then you should use == 0.
if (len == 0)
return 0;
regards,
dan carpenter
On Wed, Aug 16, 2017 at 03:06:54PM +0300, Dan Carpenter wrote:
> On Wed, Aug 16, 2017 at 12:44:51PM +0100, Colin King wrote:
> > From: Colin Ian King <[email protected]>
> >
> > The previous fix removed the equal to zero comparisons by the strcmps and
> > now the function always returns true. Fix this by adding in the missing
> > logical negation operators.
> >
> > Detected by CoverityScan, CID#1452267 ("Constant expression result")
> >
> > Fixes: b93ad9a067e1 ("staging: fsl-mc: be consistent when checking strcmp() return")
>
> Ugh... I did review the original patch at all. Sorry.
s/did/did not/
regards,
dan carpenter
On 08/16/2017 03:06 PM, Dan Carpenter wrote:
> On Wed, Aug 16, 2017 at 12:44:51PM +0100, Colin King wrote:
>> From: Colin Ian King <[email protected]>
>>
>> The previous fix removed the equal to zero comparisons by the strcmps and
>> now the function always returns true. Fix this by adding in the missing
>> logical negation operators.
>>
>> Detected by CoverityScan, CID#1452267 ("Constant expression result")
>>
>> Fixes: b93ad9a067e1 ("staging: fsl-mc: be consistent when checking strcmp() return")
Thanks Colin (and Coverity) for catching this!
> Ugh... I did review the original patch at all. Sorry.
As a side note, funny how i got the patch description right but not the
actual patch. :-)
> It's better to use "== 0" because it's idiomatic.
Agree, plus this approach would be consistent with the rest of the
driver (except one place in drivers/staging/fsl-mc/bus/dprc-driver.c +32)
---
Best Regards, Laurentiu
On 16/08/17 14:37, Laurentiu Tudor wrote:
> On 08/16/2017 03:06 PM, Dan Carpenter wrote:
>> On Wed, Aug 16, 2017 at 12:44:51PM +0100, Colin King wrote:
>>> From: Colin Ian King <[email protected]>
>>>
>>> The previous fix removed the equal to zero comparisons by the strcmps and
>>> now the function always returns true. Fix this by adding in the missing
>>> logical negation operators.
>>>
>>> Detected by CoverityScan, CID#1452267 ("Constant expression result")
>>>
>>> Fixes: b93ad9a067e1 ("staging: fsl-mc: be consistent when checking strcmp() return")
>
> Thanks Colin (and Coverity) for catching this!
>
>> Ugh... I did review the original patch at all. Sorry.
>
> As a side note, funny how i got the patch description right but not the
> actual patch. :-)
>
>> It's better to use "== 0" because it's idiomatic.
>
> Agree, plus this approach would be consistent with the rest of the
> driver (except one place in drivers/staging/fsl-mc/bus/dprc-driver.c +32)
OK, I'll send a revert sometime today as that seems the sane solution.
Colin
>
> ---
> Best Regards, Laurentiu--
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>