leandrof/linux
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

nvmet-fc: slight cleanup for kbuild test warnings

Browse Source 
The kbuild tst robot flagged the following 3 issues:

Case 1)
>> drivers/nvme/target/fc.c:1201:37: warning: Either the condition
>> '!assoc' is redundant or there is possible null pointer dereference:
>> assoc. [nullPointerRedundantCheck]
>>  struct nvmet_fc_tgtport *tgtport = assoc->tgtport;
                                       ^
>> drivers/nvme/target/fc.c:1853:7: note: Assuming that condition '!assoc'
>> is not redundant
>>   if (!assoc)
         ^
>> drivers/nvme/target/fc.c:1850:37: note: Assignment
>> 'assoc=nvmet_fc_find_target_assoc(tgtport,be64_to_cpu(
>>              rqst->associd.association_id))', assigned value is 0
>>   assoc = nvmet_fc_find_target_assoc(tgtport,
                                       ^
>> drivers/nvme/target/fc.c:1896:31: note: Calling function
>> 'nvmet_fc_delete_target_assoc', 1st argument 'assoc' value is 0
>>  nvmet_fc_delete_target_assoc(assoc);
                                 ^

The tool isn't smart enough to see that line 1854 sets a ret value which
thereafter causes the routine to exit. This occurs before any of the assoc
references, so it is not an issue. There are 2 more reportings of this
same failure.

To quiet the tool - rework the if test that does the exit to also
reference assoc.  No change in logic otherwise.

Case 2)
drivers/nvme/target/fc.c:1202:29: warning: The scope of the variable
'queue' can be reduced. [variableScope]
    struct nvmet_fc_tgt_queue *queue;
                               ^

The tool is requesting the variable be declared within the code block
that utilizes it. Ignoring this report as existing code style is fine.

Case 3)
drivers/nvme/target/fc.c:1137:16: warning: Variable 'needrandom' is
assigned a value that is never used. [unreadVariable]
       needrandom = true;
                  ^

Another parsing issue with the tool. Given that parens were not used
with the list_for_each_entry() check, it inadvertantly thinks the
break exited the outer while loop not the inner for loop.

This is not an error. But, added parens to the inner list_for_each_entry()
to quiet the tool and as it is better coding style.

-- james

Signed-off-by: James Smart <jsmart2021@gmail.com>
Reported-by: kbuild test robot <lkp@intel.com>
CC: kbuild test robot <lkp@intel.com>
CC: Christoph Hellwig <hch@lst.de>
Signed-off-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Jens Axboe <axboe@kernel.dk>
This commit is contained in:
James Smart
2020-04-06 16:55:34 -07:00
committed by Jens Axboe
parent b0012dd397
commit e4fcc72c1a
1 changed files with 4 additions and 3 deletions
Download Patch File Download Diff File Expand all files Collapse all files

7
drivers/nvme/target/fc.c
View File

@@ -1132,11 +1132,12 @@ nvmet_fc_alloc_target_assoc(struct nvmet_fc_tgtport *tgtport, void *hosthandle)
spin_lock_irqsave(&tgtport->lock, flags); spin_lock_irqsave(&tgtport->lock, flags);
needrandom = false; needrandom = false;
list_for_each_entry(tmpassoc, &tgtport->assoc_list, a_list) list_for_each_entry(tmpassoc, &tgtport->assoc_list, a_list) {
if (ran == tmpassoc->association_id) { if (ran == tmpassoc->association_id) {
needrandom = true; needrandom = true;
break; break;
} }
}
if (!needrandom) { if (!needrandom) {
assoc->association_id = ran; assoc->association_id = ran;
list_add_tail(&assoc->a_list, &tgtport->assoc_list); list_add_tail(&assoc->a_list, &tgtport->assoc_list);
@@ -1837,7 +1838,7 @@ nvmet_fc_ls_disconnect(struct nvmet_fc_tgtport *tgtport,
&iod->rqstbuf->rq_dis_assoc; &iod->rqstbuf->rq_dis_assoc;
struct fcnvme_ls_disconnect_assoc_acc *acc = struct fcnvme_ls_disconnect_assoc_acc *acc =
&iod->rspbuf->rsp_dis_assoc; &iod->rspbuf->rsp_dis_assoc;
struct nvmet_fc_tgt_assoc *assoc; struct nvmet_fc_tgt_assoc *assoc = NULL;
struct nvmet_fc_ls_iod *oldls = NULL; struct nvmet_fc_ls_iod *oldls = NULL;
unsigned long flags; unsigned long flags;
int ret = 0; int ret = 0;
@@ -1854,7 +1855,7 @@ nvmet_fc_ls_disconnect(struct nvmet_fc_tgtport *tgtport,
ret = VERR_NO_ASSOC; ret = VERR_NO_ASSOC;
} }
if (ret) { if (ret || !assoc) {
dev_err(tgtport->dev, dev_err(tgtport->dev,
"Disconnect LS failed: %s\n", "Disconnect LS failed: %s\n",
validation_errors[ret]); validation_errors[ret]);