[PATCH 0/3] hostap: Fine-tuning for a few functions

From: Markus Elfring <redacted>
Date: Sat, 20 Aug 2016 18:35:43 +0200

A few update suggestions were taken into account
from static source code analysis.

Markus Elfring (3):
  Use memdup_user()
  Delete an unnecessary jump label
  Delete unnecessary variable initialisations

 .../net/wireless/intersil/hostap/hostap_ioctl.c    | 36 ++++++++--------------
 1 file changed, 12 insertions(+), 24 deletions(-)

-- 
2.9.3

From: Markus Elfring <redacted>
Date: Sat, 20 Aug 2016 18:19:43 +0200

Reuse existing functionality from memdup_user() instead of keeping
duplicate source code.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <redacted>
---
 .../net/wireless/intersil/hostap/hostap_ioctl.c    | 22 ++++++----------------
 1 file changed, 6 insertions(+), 16 deletions(-)

diff --git a/drivers/net/wireless/intersil/hostap/hostap_ioctl.c b/drivers/net/wireless/intersil/hostap/hostap_ioctl.c
index 3e5fa78..4e271f9 100644
--- a/drivers/net/wireless/intersil/hostap/hostap_ioctl.c
+++ b/drivers/net/wireless/intersil/hostap/hostap_ioctl.c

@@ -3041,14 +3041,9 @@ static int prism2_ioctl_priv_download(local_info_t *local, struct iw_point *p)
 	    p->length > 1024 || !p->pointer)
 		return -EINVAL;
 
-	param = kmalloc(p->length, GFP_KERNEL);
-	if (param == NULL)
-		return -ENOMEM;
-
-	if (copy_from_user(param, p->pointer, p->length)) {
-		ret = -EFAULT;
-		goto out;
-	}
+	param = memdup_user(p->pointer, p->length);
+	if (IS_ERR(param))
+		return PTR_ERR(param);
 
 	if (p->length < sizeof(struct prism2_download_param) +
 	    param->num_areas * sizeof(struct prism2_download_area)) {

@@ -3803,14 +3798,9 @@ static int prism2_ioctl_priv_hostapd(local_info_t *local, struct iw_point *p)
 	    p->length > PRISM2_HOSTAPD_MAX_BUF_SIZE || !p->pointer)
 		return -EINVAL;
 
-	param = kmalloc(p->length, GFP_KERNEL);
-	if (param == NULL)
-		return -ENOMEM;
-
-	if (copy_from_user(param, p->pointer, p->length)) {
-		ret = -EFAULT;
-		goto out;
-	}
+	param = memdup_user(p->pointer, p->length);
+	if (IS_ERR(param))
+		return PTR_ERR(param);
 
 	switch (param->cmd) {
 	case PRISM2_SET_ENCRYPTION:

-- 
2.9.3

From: Markus Elfring <redacted>
Date: Sat, 20 Aug 2016 18:21:29 +0200

Remove a jump label which is unneeded in this function at the end.

Signed-off-by: Markus Elfring <redacted>
---
 drivers/net/wireless/intersil/hostap/hostap_ioctl.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/intersil/hostap/hostap_ioctl.c b/drivers/net/wireless/intersil/hostap/hostap_ioctl.c
index 4e271f9..5942917 100644
--- a/drivers/net/wireless/intersil/hostap/hostap_ioctl.c
+++ b/drivers/net/wireless/intersil/hostap/hostap_ioctl.c

@@ -3835,14 +3835,12 @@ static int prism2_ioctl_priv_hostapd(local_info_t *local, struct iw_point *p)
 	}
 
 	if (ret == 1 || !ap_ioctl) {
-		if (copy_to_user(p->pointer, param, p->length)) {
+		if (copy_to_user(p->pointer, param, p->length))
 			ret = -EFAULT;
-			goto out;
-		} else if (ap_ioctl)
+		else if (ap_ioctl)
 			ret = 0;
 	}
 
- out:
 	kfree(param);
 	return ret;
 }

-- 
2.9.3

From: Markus Elfring <redacted>
Date: Sat, 20 Aug 2016 18:23:14 +0200

The local variable "ret" will be set to an appropriate value a bit later.
Thus omit the explicit initialisation at the beginning of four functions.

Signed-off-by: Markus Elfring <redacted>
---
 drivers/net/wireless/intersil/hostap/hostap_ioctl.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/intersil/hostap/hostap_ioctl.c b/drivers/net/wireless/intersil/hostap/hostap_ioctl.c
index 5942917..c37b0bb 100644
--- a/drivers/net/wireless/intersil/hostap/hostap_ioctl.c
+++ b/drivers/net/wireless/intersil/hostap/hostap_ioctl.c

@@ -2895,7 +2895,7 @@ static int prism2_ioctl_priv_monitor(struct net_device *dev, int *i)
 {
 	struct hostap_interface *iface;
 	local_info_t *local;
-	int ret = 0;
+	int ret;
 	u32 mode;
 
 	iface = netdev_priv(dev);

@@ -3035,7 +3035,7 @@ static int ap_mac_cmd_ioctl(local_info_t *local, int *cmd)
 static int prism2_ioctl_priv_download(local_info_t *local, struct iw_point *p)
 {
 	struct prism2_download_param *param;
-	int ret = 0;
+	int ret;
 
 	if (p->length < sizeof(struct prism2_download_param) ||
 	    p->length > 1024 || !p->pointer)

@@ -3791,7 +3791,7 @@ static int prism2_ioctl_scan_req(local_info_t *local,
 static int prism2_ioctl_priv_hostapd(local_info_t *local, struct iw_point *p)
 {
 	struct prism2_hostapd_param *param;
-	int ret = 0;
+	int ret;
 	int ap_ioctl = 0;
 
 	if (p->length < sizeof(struct prism2_hostapd_param) ||

@@ -3954,7 +3954,7 @@ int hostap_ioctl(struct net_device *dev, struct ifreq *ifr, int cmd)
 	struct iwreq *wrq = (struct iwreq *) ifr;
 	struct hostap_interface *iface;
 	local_info_t *local;
-	int ret = 0;
+	int ret;
 
 	iface = netdev_priv(dev);
 	local = iface->local;

-- 
2.9.3

On 20-08-16 18:43, SF Markus Elfring wrote:

From: Markus Elfring <redacted>
Date: Sat, 20 Aug 2016 18:35:43 +0200

A few update suggestions were taken into account
from static source code analysis.

Is it worth touching this old stuff especially when you are not making
any functional changes.

Regards,
Arend

Markus Elfring (3):
  Use memdup_user()
  Delete an unnecessary jump label
  Delete unnecessary variable initialisations

 .../net/wireless/intersil/hostap/hostap_ioctl.c    | 36 ++++++++--------------
 1 file changed, 12 insertions(+), 24 deletions(-)

Hi Marcus,

On Sun, Aug 21, 2016 at 2:46 AM, SF Markus Elfring
[off-list ref] wrote:

Does this change make any difference to the compiled code?

Thanks,

-- 
Julian Calaby

Email: julian.calaby@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/

SF Markus Elfring [off-list ref] wrote:

From: Markus Elfring <redacted>
Date: Sat, 20 Aug 2016 18:19:43 +0200

Reuse existing functionality from memdup_user() instead of keeping
duplicate source code.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <redacted>

Patch set to Rejected.

[1/3] hostap: Use memdup_user() rather than duplicating i... 2016-08-20 SF Markus El Rejected

Reason: A similar patch is already applied.

Applying: hostap: Use memdup_user() rather than duplicating its implementation
Using index info to reconstruct a base tree...
Falling back to patching base and 3-way merge...
Auto-merging drivers/net/wireless/intersil/hostap/hostap_ioctl.c
CONFLICT (content): Merge conflict in drivers/net/wireless/intersil/hostap/hostap_ioctl.c
Failed to merge in the changes.
Patch failed at 0001 hostap: Use memdup_user() rather than duplicating its implementation

-- 
https://patchwork.kernel.org/patch/9306999/

Documentation about submitting wireless patches and checking status
from patchwork:

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

SF Markus Elfring [off-list ref] wrote:

From: Markus Elfring <redacted>
Date: Sat, 20 Aug 2016 18:21:29 +0200

Remove a jump label which is unneeded in this function at the end.

Signed-off-by: Markus Elfring <redacted>

2 patches set to Rejected.

9291771 [2/3] hostap: Delete an unnecessary jump label in prism2_ioctl_priv_hostapd()
9291775 [3/3] hostap: Delete unnecessary initialisations for the variable "ret"

Reason: The benefit is not clear.

-- 
https://patchwork.kernel.org/patch/9291771/

Documentation about submitting wireless patches and checking status
from patchwork:

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches