All of lore.kernel.org
 help / color / mirror / Atom feed
From: Michal Nazarewicz <mina86@mina86.com>
To: ath9k-devel@lists.ath9k.org
Subject: [ath9k-devel] [PATCH] net: wirelesse: wcn36xx: pull allocation outside of critical section
Date: Sat, 07 Dec 2013 18:13:58 +0100	[thread overview]
Message-ID: <878uvw1t49.fsf@mina86.com> (raw)
In-Reply-To: <CAFSJ42YckTDO=UV=fWG9mmVXSBYep1x6xiHahzGDcrB4dy0oOQ@mail.gmail.com>


This also simplifies flow-controll as there is now only one if
condition with a single branch.
---
 drivers/net/wireless/ath/wcn36xx/smd.c | 28 +++++++++++++++-------------
 1 file changed, 15 insertions(+), 13 deletions(-)

On Thu, Dec 05 2013, Eugene Krasnikov <k.eugene.e@gmail.com> wrote:
> I think code will look neater if error case will be handled in "else"
> statement something like: [...]

How about the following:

diff --git a/drivers/net/wireless/ath/wcn36xx/smd.c b/drivers/net/wireless/ath/wcn36xx/smd.c
index 823631c..ebab2db 100644
--- a/drivers/net/wireless/ath/wcn36xx/smd.c
+++ b/drivers/net/wireless/ath/wcn36xx/smd.c
@@ -2056,22 +2056,24 @@ static void wcn36xx_smd_rsp_process(struct wcn36xx *wcn, void *buf, size_t len)
 	case WCN36XX_HAL_OTA_TX_COMPL_IND:
 	case WCN36XX_HAL_MISSED_BEACON_IND:
 	case WCN36XX_HAL_DELETE_STA_CONTEXT_IND:
-		mutex_lock(&wcn->hal_ind_mutex);
 		msg_ind = kmalloc(sizeof(*msg_ind), GFP_KERNEL);
-		if (msg_ind) {
-			msg_ind->msg_len = len;
-			msg_ind->msg = kmalloc(len, GFP_KERNEL);
-			memcpy(msg_ind->msg, buf, len);
-			list_add_tail(&msg_ind->list, &wcn->hal_ind_queue);
-			queue_work(wcn->hal_ind_wq, &wcn->hal_ind_work);
-			wcn36xx_dbg(WCN36XX_DBG_HAL, "indication arrived\n");
+		if (!msg_ind) {
+			/*
+			 * FIXME: Do something smarter then just
+			 * printing an error.
+			 */
+			wcn36xx_err("Run out of memory while handling SMD_EVENT (%d)\n",
+				    msg_header->msg_type);
+			break;
 		}
+		mutex_lock(&wcn->hal_ind_mutex);
+		msg_ind->msg_len = len;
+		msg_ind->msg = kmalloc(len, GFP_KERNEL);
+		memcpy(msg_ind->msg, buf, len);
+		list_add_tail(&msg_ind->list, &wcn->hal_ind_queue);
+		queue_work(wcn->hal_ind_wq, &wcn->hal_ind_work);
+		wcn36xx_dbg(WCN36XX_DBG_HAL, "indication arrived\n");
 		mutex_unlock(&wcn->hal_ind_mutex);
-		if (msg_ind)
-			break;
-		/* FIXME: Do something smarter then just printing an error. */
-		wcn36xx_err("Run out of memory while handling SMD_EVENT (%d)\n",
-			    msg_header->msg_type);
 		break;
 	default:
 		wcn36xx_err("SMD_EVENT (%d) not supported\n",
-- 
1.8.4

WARNING: multiple messages have this Message-ID (diff)
From: Michal Nazarewicz <mina86@mina86.com>
To: Eugene Krasnikov <k.eugene.e@gmail.com>
Cc: Peter Stuge <peter@stuge.se>,
	"John W. Linville" <linville@tuxdriver.com>,
	ath9k-devel <ath9k-devel@lists.ath9k.org>,
	linux-wireless <linux-wireless@vger.kernel.org>,
	"linux-kernel\@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: [PATCH] net: wirelesse: wcn36xx: pull allocation outside of critical section
Date: Sat, 07 Dec 2013 18:13:58 +0100	[thread overview]
Message-ID: <878uvw1t49.fsf@mina86.com> (raw)
In-Reply-To: <CAFSJ42YckTDO=UV=fWG9mmVXSBYep1x6xiHahzGDcrB4dy0oOQ@mail.gmail.com>


This also simplifies flow-controll as there is now only one if
condition with a single branch.
---
 drivers/net/wireless/ath/wcn36xx/smd.c | 28 +++++++++++++++-------------
 1 file changed, 15 insertions(+), 13 deletions(-)

On Thu, Dec 05 2013, Eugene Krasnikov <k.eugene.e@gmail.com> wrote:
> I think code will look neater if error case will be handled in "else"
> statement something like: [...]

How about the following:

diff --git a/drivers/net/wireless/ath/wcn36xx/smd.c b/drivers/net/wireless/ath/wcn36xx/smd.c
index 823631c..ebab2db 100644
--- a/drivers/net/wireless/ath/wcn36xx/smd.c
+++ b/drivers/net/wireless/ath/wcn36xx/smd.c
@@ -2056,22 +2056,24 @@ static void wcn36xx_smd_rsp_process(struct wcn36xx *wcn, void *buf, size_t len)
 	case WCN36XX_HAL_OTA_TX_COMPL_IND:
 	case WCN36XX_HAL_MISSED_BEACON_IND:
 	case WCN36XX_HAL_DELETE_STA_CONTEXT_IND:
-		mutex_lock(&wcn->hal_ind_mutex);
 		msg_ind = kmalloc(sizeof(*msg_ind), GFP_KERNEL);
-		if (msg_ind) {
-			msg_ind->msg_len = len;
-			msg_ind->msg = kmalloc(len, GFP_KERNEL);
-			memcpy(msg_ind->msg, buf, len);
-			list_add_tail(&msg_ind->list, &wcn->hal_ind_queue);
-			queue_work(wcn->hal_ind_wq, &wcn->hal_ind_work);
-			wcn36xx_dbg(WCN36XX_DBG_HAL, "indication arrived\n");
+		if (!msg_ind) {
+			/*
+			 * FIXME: Do something smarter then just
+			 * printing an error.
+			 */
+			wcn36xx_err("Run out of memory while handling SMD_EVENT (%d)\n",
+				    msg_header->msg_type);
+			break;
 		}
+		mutex_lock(&wcn->hal_ind_mutex);
+		msg_ind->msg_len = len;
+		msg_ind->msg = kmalloc(len, GFP_KERNEL);
+		memcpy(msg_ind->msg, buf, len);
+		list_add_tail(&msg_ind->list, &wcn->hal_ind_queue);
+		queue_work(wcn->hal_ind_wq, &wcn->hal_ind_work);
+		wcn36xx_dbg(WCN36XX_DBG_HAL, "indication arrived\n");
 		mutex_unlock(&wcn->hal_ind_mutex);
-		if (msg_ind)
-			break;
-		/* FIXME: Do something smarter then just printing an error. */
-		wcn36xx_err("Run out of memory while handling SMD_EVENT (%d)\n",
-			    msg_header->msg_type);
 		break;
 	default:
 		wcn36xx_err("SMD_EVENT (%d) not supported\n",
-- 
1.8.4


WARNING: multiple messages have this Message-ID (diff)
From: Michal Nazarewicz <mina86@mina86.com>
To: Eugene Krasnikov <k.eugene.e@gmail.com>
Cc: Peter Stuge <peter@stuge.se>,
	"John W. Linville" <linville@tuxdriver.com>,
	ath9k-devel <ath9k-devel@venema.h4ckr.net>,
	linux-wireless <linux-wireless@vger.kernel.org>,
	"linux-kernel\@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: [PATCH] net: wirelesse: wcn36xx: pull allocation outside of critical section
Date: Sat, 07 Dec 2013 18:13:58 +0100	[thread overview]
Message-ID: <878uvw1t49.fsf@mina86.com> (raw)
In-Reply-To: <CAFSJ42YckTDO=UV=fWG9mmVXSBYep1x6xiHahzGDcrB4dy0oOQ@mail.gmail.com>


This also simplifies flow-controll as there is now only one if
condition with a single branch.
---
 drivers/net/wireless/ath/wcn36xx/smd.c | 28 +++++++++++++++-------------
 1 file changed, 15 insertions(+), 13 deletions(-)

On Thu, Dec 05 2013, Eugene Krasnikov <k.eugene.e@gmail.com> wrote:
> I think code will look neater if error case will be handled in "else"
> statement something like: [...]

How about the following:

diff --git a/drivers/net/wireless/ath/wcn36xx/smd.c b/drivers/net/wireless/ath/wcn36xx/smd.c
index 823631c..ebab2db 100644
--- a/drivers/net/wireless/ath/wcn36xx/smd.c
+++ b/drivers/net/wireless/ath/wcn36xx/smd.c
@@ -2056,22 +2056,24 @@ static void wcn36xx_smd_rsp_process(struct wcn36xx *wcn, void *buf, size_t len)
 	case WCN36XX_HAL_OTA_TX_COMPL_IND:
 	case WCN36XX_HAL_MISSED_BEACON_IND:
 	case WCN36XX_HAL_DELETE_STA_CONTEXT_IND:
-		mutex_lock(&wcn->hal_ind_mutex);
 		msg_ind = kmalloc(sizeof(*msg_ind), GFP_KERNEL);
-		if (msg_ind) {
-			msg_ind->msg_len = len;
-			msg_ind->msg = kmalloc(len, GFP_KERNEL);
-			memcpy(msg_ind->msg, buf, len);
-			list_add_tail(&msg_ind->list, &wcn->hal_ind_queue);
-			queue_work(wcn->hal_ind_wq, &wcn->hal_ind_work);
-			wcn36xx_dbg(WCN36XX_DBG_HAL, "indication arrived\n");
+		if (!msg_ind) {
+			/*
+			 * FIXME: Do something smarter then just
+			 * printing an error.
+			 */
+			wcn36xx_err("Run out of memory while handling SMD_EVENT (%d)\n",
+				    msg_header->msg_type);
+			break;
 		}
+		mutex_lock(&wcn->hal_ind_mutex);
+		msg_ind->msg_len = len;
+		msg_ind->msg = kmalloc(len, GFP_KERNEL);
+		memcpy(msg_ind->msg, buf, len);
+		list_add_tail(&msg_ind->list, &wcn->hal_ind_queue);
+		queue_work(wcn->hal_ind_wq, &wcn->hal_ind_work);
+		wcn36xx_dbg(WCN36XX_DBG_HAL, "indication arrived\n");
 		mutex_unlock(&wcn->hal_ind_mutex);
-		if (msg_ind)
-			break;
-		/* FIXME: Do something smarter then just printing an error. */
-		wcn36xx_err("Run out of memory while handling SMD_EVENT (%d)\n",
-			    msg_header->msg_type);
 		break;
 	default:
 		wcn36xx_err("SMD_EVENT (%d) not supported\n",
-- 
1.8.4


  parent reply	other threads:[~2013-12-07 17:13 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-11-29 17:07 [ath9k-devel] [PATCH] net: wireless: wcn36xx: fix potential NULL pointer dereference Michal Nazarewicz
2013-11-29 17:07 ` Michal Nazarewicz
2013-11-29 17:07 ` Michal Nazarewicz
2013-12-01  0:47 ` [ath9k-devel] " Peter Stuge
2013-12-01  0:47   ` Peter Stuge
2013-12-01  0:47   ` Peter Stuge
2013-12-02 13:09   ` [ath9k-devel] [PATCHv2] " Michal Nazarewicz
2013-12-02 13:09     ` Michal Nazarewicz
2013-12-02 13:09     ` Michal Nazarewicz
2013-12-05  8:41     ` [ath9k-devel] " Eugene Krasnikov
2013-12-05  8:41       ` Eugene Krasnikov
2013-12-05  8:41       ` Eugene Krasnikov
2013-12-06 16:12       ` [ath9k-devel] " John W. Linville
2013-12-06 16:12         ` John W. Linville
2013-12-07 17:13       ` Michal Nazarewicz [this message]
2013-12-07 17:13         ` [PATCH] net: wirelesse: wcn36xx: pull allocation outside of critical section Michal Nazarewicz
2013-12-07 17:13         ` Michal Nazarewicz
2013-12-09  7:27         ` [ath9k-devel] " Eugene Krasnikov
2013-12-09  7:27           ` Eugene Krasnikov
2013-12-09  7:27           ` Eugene Krasnikov
2013-12-09 11:34           ` [ath9k-devel] [PATCH] net: wirelesse: wcn36xx: check allocation and trim " Michal Nazarewicz
2013-12-09 11:34             ` Michal Nazarewicz
2013-12-09 11:34             ` Michal Nazarewicz
2013-12-11 15:52             ` [ath9k-devel] " John W. Linville
2013-12-11 15:52               ` John W. Linville
2013-12-11 16:43               ` [ath9k-devel] [PATCH RESEND] net: wirelesse: wcn36xx: pull allocation outside of " Michal Nazarewicz
2013-12-11 16:43                 ` Michal Nazarewicz

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=878uvw1t49.fsf@mina86.com \
    --to=mina86@mina86.com \
    --cc=ath9k-devel@lists.ath9k.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.