kernel-janitors.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Dan Carpenter <dan.carpenter@oracle.com>
To: "James E.J. Bottomley" <jejb@linux.ibm.com>
Cc: "Martin K. Petersen" <martin.petersen@oracle.com>,
	Tomas Henzl <thenzl@redhat.com>, Hannes Reinecke <hare@suse.de>,
	Kashyap Desai <kashyap.desai@broadcom.com>,
	Himanshu Madhani <himanshu.madhani@oracle.com>,
	linux-scsi@vger.kernel.org, kernel-janitors@vger.kernel.org
Subject: [PATCH 2/2] scsi: mpi3mr: Fix error handling in mpi3mr_setup_isr()
Date: Wed, 9 Jun 2021 12:27:14 +0300	[thread overview]
Message-ID: <YMCJcnmSI4kOIyv/@mwanda> (raw)
In-Reply-To: <YMCJKgykDYtyvY44@mwanda>

The pci_alloc_irq_vectors_affinity() function returns negative error
codes or it returns a number between the minimum vectors (1 in this
case) and max_vectors.  It won't return zero.  Because "i" is a u16 then
the error handling won't work.  And also if it did work the error code
was not set.

Really "max_vectors" can be an int as well because we're doing a min_t()
on int type.  The other change is that it's better to remove unnecessary
initialization so that static checkers can warn us if there are ever
uninitialized variable bugs introduced in the future.

I changed the error code from -1 (-EPERM) if the kmalloc() failed to
-ENOMEM.  And on success path I changed it from "return retval;" to
"return 0;" which shouldn't affect the compiled code but makes it more
readable.

Fixes: 824a156633df ("scsi: mpi3mr: Base driver code")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
 drivers/scsi/mpi3mr/mpi3mr_fw.c | 24 +++++++++++++-----------
 1 file changed, 13 insertions(+), 11 deletions(-)

diff --git a/drivers/scsi/mpi3mr/mpi3mr_fw.c b/drivers/scsi/mpi3mr/mpi3mr_fw.c
index 40696b75345d..88db2f0e13fd 100644
--- a/drivers/scsi/mpi3mr/mpi3mr_fw.c
+++ b/drivers/scsi/mpi3mr/mpi3mr_fw.c
@@ -665,8 +665,9 @@ static inline int mpi3mr_request_irq(struct mpi3mr_ioc *mrioc, u16 index)
 static int mpi3mr_setup_isr(struct mpi3mr_ioc *mrioc, u8 setup_one)
 {
 	unsigned int irq_flags = PCI_IRQ_MSIX;
-	u16 max_vectors = 0, i;
-	int retval = 0;
+	int max_vectors;
+	int retval;
+	int i;
 	struct irq_affinity desc = { .pre_vectors =  1};
 
 	mpi3mr_cleanup_isr(mrioc);
@@ -687,29 +688,29 @@ static int mpi3mr_setup_isr(struct mpi3mr_ioc *mrioc, u8 setup_one)
 	irq_flags |= PCI_IRQ_AFFINITY | PCI_IRQ_ALL_TYPES;
 
 	mrioc->op_reply_q_offset = (max_vectors > 1) ? 1 : 0;
-	i = pci_alloc_irq_vectors_affinity(mrioc->pdev,
-	    1, max_vectors, irq_flags, &desc);
-	if (i <= 0) {
+	retval = pci_alloc_irq_vectors_affinity(mrioc->pdev,
+				1, max_vectors, irq_flags, &desc);
+	if (retval < 0) {
 		ioc_err(mrioc, "Cannot alloc irq vectors\n");
 		goto out_failed;
 	}
-	if (i != max_vectors) {
+	if (retval != max_vectors) {
 		ioc_info(mrioc,
 		    "allocated vectors (%d) are less than configured (%d)\n",
-		    i, max_vectors);
+		    retval, max_vectors);
 		/*
 		 * If only one MSI-x is allocated, then MSI-x 0 will be shared
 		 * between Admin queue and operational queue
 		 */
-		if (i == 1)
+		if (retval == 1)
 			mrioc->op_reply_q_offset = 0;
 
-		max_vectors = i;
+		max_vectors = retval;
 	}
 	mrioc->intr_info = kzalloc(sizeof(struct mpi3mr_intr_info) * max_vectors,
 	    GFP_KERNEL);
 	if (!mrioc->intr_info) {
-		retval = -1;
+		retval = -ENOMEM;
 		pci_free_irq_vectors(mrioc->pdev);
 		goto out_failed;
 	}
@@ -722,7 +723,8 @@ static int mpi3mr_setup_isr(struct mpi3mr_ioc *mrioc, u8 setup_one)
 	}
 	mrioc->intr_info_count = max_vectors;
 	mpi3mr_ioc_enable_intr(mrioc);
-	return retval;
+	return 0;
+
 out_failed:
 	mpi3mr_cleanup_isr(mrioc);
 
-- 
2.30.2


  reply	other threads:[~2021-06-09  9:27 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-09  9:26 [PATCH 1/2] scsi: mpi3mr: delete unnecessary NULL check Dan Carpenter
2021-06-09  9:27 ` Dan Carpenter [this message]
2021-06-10  3:01 ` Martin K. Petersen
2021-06-16  3:48 ` Martin K. Petersen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YMCJcnmSI4kOIyv/@mwanda \
    --to=dan.carpenter@oracle.com \
    --cc=hare@suse.de \
    --cc=himanshu.madhani@oracle.com \
    --cc=jejb@linux.ibm.com \
    --cc=kashyap.desai@broadcom.com \
    --cc=kernel-janitors@vger.kernel.org \
    --cc=linux-scsi@vger.kernel.org \
    --cc=martin.petersen@oracle.com \
    --cc=thenzl@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).