[patch] nvme: precedence bug in nvme_pr_clear()

[patch] nvme: precedence bug in nvme_pr_clear()

[Dan Carpenter]

The "|" operator has higher precedence than "?:" so this didn't work as
intended.  I had previously fixed this bug, but it we copied the older
unfixed version when we moved the function between files.

Fixes: 1673f1f08c88 ('nvme: move block_device_operations and ns/ctrl freeing to common code')
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index f9c4e80..b8dc123 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -671,7 +671,7 @@ static int nvme_pr_preempt(struct block_device *bdev, u64 old, u64 new,
 
 static int nvme_pr_clear(struct block_device *bdev, u64 key)
 {
-	u32 cdw10 = 1 | key ? 1 << 3 : 0;
+	u32 cdw10 = 1 | (key ? 1 << 3 : 0);
 	return nvme_pr_command(bdev, cdw10, key, 0, nvme_cmd_resv_register);
 }
 

Re: [patch] nvme: precedence bug in nvme_pr_clear()

[Jens Axboe]

On 12/09/2015 03:24 AM, Dan Carpenter wrote:
> The "|" operator has higher precedence than "?:" so this didn't work as
> intended.  I had previously fixed this bug, but it we copied the older
> unfixed version when we moved the function between files.
>
> Fixes: 1673f1f08c88 ('nvme: move block_device_operations and ns/ctrl freeing to common code')
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

Dejavu, but I guess in a different function. Christoph, you are hereby 
banned from ever using ?:!

-- 
Jens Axboe

Re: [patch] nvme: precedence bug in nvme_pr_clear()

[Christoph Hellwig]

On Wed, Dec 09, 2015 at 10:56:37AM -0700, Jens Axboe wrote:
> On 12/09/2015 03:24 AM, Dan Carpenter wrote:
>> The "|" operator has higher precedence than "?:" so this didn't work as
>> intended.  I had previously fixed this bug, but it we copied the older
>> unfixed version when we moved the function between files.
>>
>> Fixes: 1673f1f08c88 ('nvme: move block_device_operations and ns/ctrl freeing to common code')
>> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
>
> Dejavu, but I guess in a different function. Christoph, you are hereby 
> banned from ever using ?:!

Nah, I'm banned from ever moving large chunks of code around again :)
The orginal Bug is Keith's, I just undid the fix by not properly fixing
up the code move for the conflict.

Re: [patch] nvme: precedence bug in nvme_pr_clear()

[Jens Axboe]

On 12/09/2015 11:00 AM, Christoph Hellwig wrote:
> On Wed, Dec 09, 2015 at 10:56:37AM -0700, Jens Axboe wrote:
>> On 12/09/2015 03:24 AM, Dan Carpenter wrote:
>>> The "|" operator has higher precedence than "?:" so this didn't work as
>>> intended.  I had previously fixed this bug, but it we copied the older
>>> unfixed version when we moved the function between files.
>>>
>>> Fixes: 1673f1f08c88 ('nvme: move block_device_operations and ns/ctrl freeing to common code')
>>> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
>>
>> Dejavu, but I guess in a different function. Christoph, you are hereby
>> banned from ever using ?:!
>
> Nah, I'm banned from ever moving large chunks of code around again :)
> The orginal Bug is Keith's, I just undid the fix by not properly fixing
> up the code move for the conflict.

Alright you are off the hook, the ban will be instated in Keith instead :-)

-- 
Jens Axboe

Re: [patch] nvme: precedence bug in nvme_pr_clear()

[Keith Busch]

On Wed, Dec 09, 2015 at 11:05:06AM -0700, Jens Axboe wrote:
> 
> Alright you are off the hook, the ban will be instated in Keith instead :-)

*humbled*

;)

Re: [patch] nvme: precedence bug in nvme_pr_clear()

[Keith Busch]

On Wed, Dec 09, 2015 at 10:56:37AM -0700, Jens Axboe wrote:
> On 12/09/2015 03:24 AM, Dan Carpenter wrote:
> >The "|" operator has higher precedence than "?:" so this didn't work as
> >intended.  I had previously fixed this bug, but it we copied the older
> >unfixed version when we moved the function between files.
> >
> >Fixes: 1673f1f08c88 ('nvme: move block_device_operations and ns/ctrl freeing to common code')
> >Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> 
> Dejavu, but I guess in a different function. Christoph, you are
> hereby banned from ever using ?:!

Ah, that came from me. I wrote the patch on one machine, tested on
another. The tests required those issues be fixed, but the fixes didn't
propgate back to the original machine that generated the patch.

git user fail...

Re: [patch] nvme: precedence bug in nvme_pr_clear()

[Dan Carpenter]

On Wed, Dec 09, 2015 at 10:56:37AM -0700, Jens Axboe wrote:
> On 12/09/2015 03:24 AM, Dan Carpenter wrote:
> >The "|" operator has higher precedence than "?:" so this didn't work as
> >intended.  I had previously fixed this bug, but it we copied the older
> >unfixed version when we moved the function between files.
> >
> >Fixes: 1673f1f08c88 ('nvme: move block_device_operations and ns/ctrl freeing to common code')
> >Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> 
> Dejavu, but I guess in a different function. Christoph, you are
> hereby banned from ever using ?:!

Christoph didn't write this.  I think where it went wrong is:

[Moved the integrity and pr changes due to merge conflict]

We merged the buggy version instead of the fixed version.  I don't know
git well enough to be positive.

regards,
dan carpenter