public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Dan Carpenter <dan.carpenter@oracle.com>
To: Rupesh Gujare <rupesh.gujare@atmel.com>
Cc: devel@linuxdriverproject.org, linux-usb@vger.kernel.org,
	linux-kernel@vger.kernel.org, gregkh@linuxfoundation.org
Subject: Re: [PATCH 2/4] staging: ozwpan: Increment port number for new device.
Date: Mon, 5 Aug 2013 23:33:40 +0300	[thread overview]
Message-ID: <20130805203340.GP5051@mwanda> (raw)
In-Reply-To: <20130805202338.GO5051@mwanda>

Here is what oz_hcd_pd_arrived() looks like after my changes:

These lines:
-       if (ozhcd == NULL)
+       if (!ozhcd)
were personal preference and not official kernel style guidelines.
Either way is fine for pointers.

oz_ep_alloc() can return NULL.  There was no check for failure in
the original code.

diff --git a/drivers/staging/ozwpan/ozhcd.c b/drivers/staging/ozwpan/ozhcd.c
index f81a0c5..6b9fc02 100644
--- a/drivers/staging/ozwpan/ozhcd.c
+++ b/drivers/staging/ozwpan/ozhcd.c
@@ -620,58 +621,59 @@ static inline void oz_hcd_put(struct oz_hcd *ozhcd)
  * probably very rare indeed.
  * Context: softirq
  */
-void *oz_hcd_pd_arrived(void *hpd)
+struct oz_port *oz_hcd_pd_arrived(void *hpd)
 {
-	int i;
-	void *hport = NULL;
-	struct oz_hcd *ozhcd = NULL;
+	struct oz_port *hport = NULL;
+	struct oz_hcd *ozhcd;
 	struct oz_endpoint *ep;
+	int i;
+
 	ozhcd = oz_hcd_claim();
-	if (ozhcd == NULL)
+	if (!ozhcd)
 		return NULL;
-	/* Allocate an endpoint object in advance (before holding hcd lock) to
-	 * use for out endpoint 0.
-	 */
+
+	/* Allocate an endpoint object to use for out endpoint 0. */
 	ep = oz_ep_alloc(GFP_ATOMIC, 0);
+	if (!ep)
+		goto err_put;
+
 	spin_lock_bh(&ozhcd->hcd_lock);
-	if (ozhcd->conn_port >= 0) {
-		spin_unlock_bh(&ozhcd->hcd_lock);
-		oz_dbg(ON, "conn_port >= 0\n");
-		goto out;
-	}
+	if (ozhcd->conn_port >= 0)
+		goto err_unlock;
+
 	for (i = 0; i < OZ_NB_PORTS; i++) {
 		struct oz_port *port = &ozhcd->ports[i];
+
 		spin_lock(&port->port_lock);
-		if ((port->flags & OZ_PORT_F_PRESENT) == 0) {
+		if (!(port->flags & OZ_PORT_F_PRESENT)) {
 			oz_acquire_port(port, hpd);
 			spin_unlock(&port->port_lock);
 			break;
 		}
 		spin_unlock(&port->port_lock);
 	}
-	if (i < OZ_NB_PORTS) {
-		oz_dbg(ON, "Setting conn_port = %d\n", i);
-		ozhcd->conn_port = i;
-		/* Attach out endpoint 0.
-		 */
-		ozhcd->ports[i].out_ep[0] = ep;
-		ep = NULL;
-		hport = &ozhcd->ports[i];
-		spin_unlock_bh(&ozhcd->hcd_lock);
-		if (ozhcd->flags & OZ_HDC_F_SUSPENDED) {
-			oz_dbg(ON, "Resuming root hub\n");
-			usb_hcd_resume_root_hub(ozhcd->hcd);
-		}
-		usb_hcd_poll_rh_status(ozhcd->hcd);
-	} else {
-		spin_unlock_bh(&ozhcd->hcd_lock);
-	}
-out:
-	if (ep) /* ep is non-null if not used. */
-		oz_ep_free(NULL, ep);
+	if (i == OZ_NB_PORTS)
+		goto err_unlock;
+
+	ozhcd->conn_port = i;
+	hport = &ozhcd->ports[i];
+	hport->out_ep[0] = ep;
+	spin_unlock_bh(&ozhcd->hcd_lock);
+	if (ozhcd->flags & OZ_HDC_F_SUSPENDED)
+		usb_hcd_resume_root_hub(ozhcd->hcd);
+	usb_hcd_poll_rh_status(ozhcd->hcd);
 	oz_hcd_put(ozhcd);
+
 	return hport;
+
+err_unlock:
+	spin_unlock_bh(&ozhcd->hcd_lock);
+	oz_ep_free(NULL, ep);
+err_put:
+	oz_hcd_put(ozhcd);
+	return NULL;
 }
+
 /*------------------------------------------------------------------------------
  * This is called by the protocol handler to notify that the PD has gone away.
  * We need to deallocate all resources and then request that the root hub is

  reply	other threads:[~2013-08-05 20:34 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-08-05 17:40 [PATCH 0/4] staging: ozwpan: Fix crash issues Rupesh Gujare
2013-08-05 17:40 ` [PATCH 1/4] staging: ozwpan: Fixes crash due to invalid port aceess Rupesh Gujare
2013-08-05 18:20   ` Dan Carpenter
2013-08-05 17:40 ` [PATCH 2/4] staging: ozwpan: Increment port number for new device Rupesh Gujare
2013-08-05 17:53   ` Dan Carpenter
2013-08-05 18:43     ` Rupesh Gujare
2013-08-05 20:23       ` Dan Carpenter
2013-08-05 20:33         ` Dan Carpenter [this message]
2013-08-06 10:26         ` Rupesh Gujare
2013-08-06 10:41           ` Dan Carpenter
2013-08-12 21:57   ` Greg KH
2013-08-05 17:40 ` [PATCH 3/4] staging: ozwpan: Reset port configuration number Rupesh Gujare
2013-08-05 18:21   ` Dan Carpenter
2013-08-05 18:58     ` Rupesh Gujare
2013-08-05 17:40 ` [PATCH 4/4] staging: ozwpan: Return correct hub status Rupesh Gujare
2013-08-05 18:56   ` Dan Carpenter
2013-08-05 19:00     ` Rupesh Gujare
2013-08-05 19:28       ` Dan Carpenter

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=20130805203340.GP5051@mwanda \
    --to=dan.carpenter@oracle.com \
    --cc=devel@linuxdriverproject.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=rupesh.gujare@atmel.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