Skip to contentSkip to conversation

nvme: precedence bug in nvme_pr_clear()

PATCH In review Dan Carpenter 4 participants 7 messages Dec 9, 2015

Subsystems (2)
Kernel janitors NVMe
lore
Share & tools
lore

Dan Carpenter

author
#0 lore raw

PATCH 1/1 nvme: precedence bug in nvme_pr_clear()

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 <[email protected]>
drivers/nvme/host/core.c +1 -1
index f9c4e80..b8dc123 100644
@@ -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);
 }
#1 lore raw
Quoted 6 lines · 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 <[email protected]>

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

Signature · 1 line
Jens Axboe
maintainer
#2 lore raw
Quoted 10 lines · Jens Axboe
Quoted 6 lines · 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 <[email protected]>

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.

#5 lore raw
In reply to Jens Axboe
Quoted 10 lines · Jens Axboe
Quoted 6 lines · 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 <[email protected]>

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...

#3 lore raw
In reply to Christoph Hellwig
Quoted 9 lines · Christoph Hellwig
Quoted 8 lines · Jens Axboe
Quoted 6 lines · 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 <[email protected]>
Quoted 6 lines
Quoted 2 lines

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 :-)

Signature · 1 line
Jens Axboe
author
#6 lore raw
In reply to Jens Axboe
Quoted 10 lines · Jens Axboe
Quoted 6 lines · 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 <[email protected]>

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