All of lore.kernel.org
 help / color / mirror / Atom feed
From: Peter Huewe <peterhuewe@gmx.de>
To: Samuel Ortiz <sameo@linux.intel.com>,
	Alexey Dobriyan <adobriyan@gmail.com>
Cc: Srinidhi Kasagar <srinidhi.kasagar@stericsson.com>,
	Linus Walleij <linus.walleij@stericsson.com>,
	Mark Brown <broonie@opensource.wolfsonmicro.com>,
	Ian Lartey <ian@opensource.wolfsonmicro.com>,
	Dimitris Papastamos <dp@opensource.wolfsonmicro.com>,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, kernel-janitors@vger.kernel.org,
	Peter Huewe <peterhuewe@gmx.de>
Subject: [PATCH 1/2 v3] mfd/ab3550: Convert to kstrtou8_from_user
Date: Mon, 20 Jun 2011 23:01:37 +0200	[thread overview]
Message-ID: <1308603698-32166-1-git-send-email-peterhuewe@gmx.de> (raw)
In-Reply-To: <20110620195037.GA2534@p183.telecom.by>

This patch replaces the code for getting an number from a
userspace buffer by a simple call to kstrou8_from_user.
This makes it easier to read and less error prone.

Since the old buffers held only values up to 255, we don't need
kstrtoul, but rather kstrtou8.

Kernel Version: v3.0-rc3

Changes in v2:
- Use kstrtou8 instead of kstrtoul due to small numbers
- Dropped then unnecessary checks
Changes in v3:
- Drop now unnecessary local variables
- Change the debug_address and debug_bank members to u8 since they hold
  only values up to 255, and also remove the now superfluous u8 casts
(Remarks from Alexey Dobriyan, Thanks!)

Signed-off-by: Peter Huewe <peterhuewe@gmx.de>
---
 drivers/mfd/ab3550-core.c |   79 +++++++++++++-------------------------------
 1 files changed, 24 insertions(+), 55 deletions(-)

diff --git a/drivers/mfd/ab3550-core.c b/drivers/mfd/ab3550-core.c
index 3d7dce6..bd0df90 100644
--- a/drivers/mfd/ab3550-core.c
+++ b/drivers/mfd/ab3550-core.c
@@ -73,8 +73,8 @@ struct ab3550 {
 	u8 startup_events[AB3550_NUM_EVENT_REG];
 	bool startup_events_read;
 #ifdef CONFIG_DEBUG_FS
-	unsigned int debug_bank;
-	unsigned int debug_address;
+	u8 debug_bank;
+	u8 debug_address;
 #endif
 };
 
@@ -879,20 +879,13 @@ static ssize_t ab3550_bank_write(struct file *file,
 	size_t count, loff_t *ppos)
 {
 	struct ab3550 *ab = ((struct seq_file *)(file->private_data))->private;
-	char buf[32];
-	int buf_size;
-	unsigned long user_bank;
+	u8 user_bank;
 	int err;
 
-	/* Get userspace string and assure termination */
-	buf_size = min(count, (sizeof(buf) - 1));
-	if (copy_from_user(buf, user_buf, buf_size))
-		return -EFAULT;
-	buf[buf_size] = 0;
-
-	err = strict_strtoul(buf, 0, &user_bank);
+	/* Get userspace string and convert to number */
+	err = kstrtou8_from_user(user_buf, count, 0, &user_bank);
 	if (err)
-		return -EINVAL;
+		return err;
 
 	if (user_bank >= AB3550_NUM_BANKS) {
 		dev_err(&ab->i2c_client[0]->dev,
@@ -902,7 +895,7 @@ static ssize_t ab3550_bank_write(struct file *file,
 
 	ab->debug_bank = user_bank;
 
-	return buf_size;
+	return count;
 }
 
 static int ab3550_address_print(struct seq_file *s, void *p)
@@ -923,27 +916,14 @@ static ssize_t ab3550_address_write(struct file *file,
 	size_t count, loff_t *ppos)
 {
 	struct ab3550 *ab = ((struct seq_file *)(file->private_data))->private;
-	char buf[32];
-	int buf_size;
-	unsigned long user_address;
 	int err;
 
-	/* Get userspace string and assure termination */
-	buf_size = min(count, (sizeof(buf) - 1));
-	if (copy_from_user(buf, user_buf, buf_size))
-		return -EFAULT;
-	buf[buf_size] = 0;
-
-	err = strict_strtoul(buf, 0, &user_address);
+	/* Get userspace string and convert to number */
+	err = kstrtou8_from_user(user_buf, count, 0, &ab->debug_address);
 	if (err)
-		return -EINVAL;
-	if (user_address > 0xff) {
-		dev_err(&ab->i2c_client[0]->dev,
-			"debugfs error input > 0xff\n");
-		return -EINVAL;
-	}
-	ab->debug_address = user_address;
-	return buf_size;
+		return err;
+
+	return count;
 }
 
 static int ab3550_val_print(struct seq_file *s, void *p)
@@ -952,8 +932,8 @@ static int ab3550_val_print(struct seq_file *s, void *p)
 	int err;
 	u8 regvalue;
 
-	err = get_register_interruptible(ab, (u8)ab->debug_bank,
-		(u8)ab->debug_address, &regvalue);
+	err = get_register_interruptible(ab, ab->debug_bank,
+		ab->debug_address, &regvalue);
 	if (err)
 		return -EINVAL;
 	seq_printf(s, "0x%02X\n", regvalue);
@@ -971,38 +951,27 @@ static ssize_t ab3550_val_write(struct file *file,
 	size_t count, loff_t *ppos)
 {
 	struct ab3550 *ab = ((struct seq_file *)(file->private_data))->private;
-	char buf[32];
-	int buf_size;
-	unsigned long user_val;
+	u8 user_val;
 	int err;
 	u8 regvalue;
 
-	/* Get userspace string and assure termination */
-	buf_size = min(count, (sizeof(buf)-1));
-	if (copy_from_user(buf, user_buf, buf_size))
-		return -EFAULT;
-	buf[buf_size] = 0;
-
-	err = strict_strtoul(buf, 0, &user_val);
+	/* Get userspace string and convert to number */
+	err = kstrtou8_from_user(user_buf, count, 0, &user_val);
 	if (err)
-		return -EINVAL;
-	if (user_val > 0xff) {
-		dev_err(&ab->i2c_client[0]->dev,
-			"debugfs error input > 0xff\n");
-		return -EINVAL;
-	}
+		return err;
+
 	err = mask_and_set_register_interruptible(
-		ab, (u8)ab->debug_bank,
-		(u8)ab->debug_address, 0xFF, (u8)user_val);
+		ab, ab->debug_bank,
+		ab->debug_address, 0xFF, user_val);
 	if (err)
 		return -EINVAL;
 
-	get_register_interruptible(ab, (u8)ab->debug_bank,
-		(u8)ab->debug_address, &regvalue);
+	get_register_interruptible(ab, ab->debug_bank,
+		ab->debug_address, &regvalue);
 	if (err)
 		return -EINVAL;
 
-	return buf_size;
+	return count;
 }
 
 static const struct file_operations ab3550_bank_fops = {
-- 
1.7.3.4


WARNING: multiple messages have this Message-ID (diff)
From: Peter Huewe <peterhuewe@gmx.de>
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 1/2 v3] mfd/ab3550: Convert to kstrtou8_from_user
Date: Mon, 20 Jun 2011 21:01:37 +0000	[thread overview]
Message-ID: <1308603698-32166-1-git-send-email-peterhuewe@gmx.de> (raw)
In-Reply-To: <20110620195037.GA2534@p183.telecom.by>

This patch replaces the code for getting an number from a
userspace buffer by a simple call to kstrou8_from_user.
This makes it easier to read and less error prone.

Since the old buffers held only values up to 255, we don't need
kstrtoul, but rather kstrtou8.

Kernel Version: v3.0-rc3

Changes in v2:
- Use kstrtou8 instead of kstrtoul due to small numbers
- Dropped then unnecessary checks
Changes in v3:
- Drop now unnecessary local variables
- Change the debug_address and debug_bank members to u8 since they hold
  only values up to 255, and also remove the now superfluous u8 casts
(Remarks from Alexey Dobriyan, Thanks!)

Signed-off-by: Peter Huewe <peterhuewe@gmx.de>
---
 drivers/mfd/ab3550-core.c |   79 +++++++++++++-------------------------------
 1 files changed, 24 insertions(+), 55 deletions(-)

diff --git a/drivers/mfd/ab3550-core.c b/drivers/mfd/ab3550-core.c
index 3d7dce6..bd0df90 100644
--- a/drivers/mfd/ab3550-core.c
+++ b/drivers/mfd/ab3550-core.c
@@ -73,8 +73,8 @@ struct ab3550 {
 	u8 startup_events[AB3550_NUM_EVENT_REG];
 	bool startup_events_read;
 #ifdef CONFIG_DEBUG_FS
-	unsigned int debug_bank;
-	unsigned int debug_address;
+	u8 debug_bank;
+	u8 debug_address;
 #endif
 };
 
@@ -879,20 +879,13 @@ static ssize_t ab3550_bank_write(struct file *file,
 	size_t count, loff_t *ppos)
 {
 	struct ab3550 *ab = ((struct seq_file *)(file->private_data))->private;
-	char buf[32];
-	int buf_size;
-	unsigned long user_bank;
+	u8 user_bank;
 	int err;
 
-	/* Get userspace string and assure termination */
-	buf_size = min(count, (sizeof(buf) - 1));
-	if (copy_from_user(buf, user_buf, buf_size))
-		return -EFAULT;
-	buf[buf_size] = 0;
-
-	err = strict_strtoul(buf, 0, &user_bank);
+	/* Get userspace string and convert to number */
+	err = kstrtou8_from_user(user_buf, count, 0, &user_bank);
 	if (err)
-		return -EINVAL;
+		return err;
 
 	if (user_bank >= AB3550_NUM_BANKS) {
 		dev_err(&ab->i2c_client[0]->dev,
@@ -902,7 +895,7 @@ static ssize_t ab3550_bank_write(struct file *file,
 
 	ab->debug_bank = user_bank;
 
-	return buf_size;
+	return count;
 }
 
 static int ab3550_address_print(struct seq_file *s, void *p)
@@ -923,27 +916,14 @@ static ssize_t ab3550_address_write(struct file *file,
 	size_t count, loff_t *ppos)
 {
 	struct ab3550 *ab = ((struct seq_file *)(file->private_data))->private;
-	char buf[32];
-	int buf_size;
-	unsigned long user_address;
 	int err;
 
-	/* Get userspace string and assure termination */
-	buf_size = min(count, (sizeof(buf) - 1));
-	if (copy_from_user(buf, user_buf, buf_size))
-		return -EFAULT;
-	buf[buf_size] = 0;
-
-	err = strict_strtoul(buf, 0, &user_address);
+	/* Get userspace string and convert to number */
+	err = kstrtou8_from_user(user_buf, count, 0, &ab->debug_address);
 	if (err)
-		return -EINVAL;
-	if (user_address > 0xff) {
-		dev_err(&ab->i2c_client[0]->dev,
-			"debugfs error input > 0xff\n");
-		return -EINVAL;
-	}
-	ab->debug_address = user_address;
-	return buf_size;
+		return err;
+
+	return count;
 }
 
 static int ab3550_val_print(struct seq_file *s, void *p)
@@ -952,8 +932,8 @@ static int ab3550_val_print(struct seq_file *s, void *p)
 	int err;
 	u8 regvalue;
 
-	err = get_register_interruptible(ab, (u8)ab->debug_bank,
-		(u8)ab->debug_address, &regvalue);
+	err = get_register_interruptible(ab, ab->debug_bank,
+		ab->debug_address, &regvalue);
 	if (err)
 		return -EINVAL;
 	seq_printf(s, "0x%02X\n", regvalue);
@@ -971,38 +951,27 @@ static ssize_t ab3550_val_write(struct file *file,
 	size_t count, loff_t *ppos)
 {
 	struct ab3550 *ab = ((struct seq_file *)(file->private_data))->private;
-	char buf[32];
-	int buf_size;
-	unsigned long user_val;
+	u8 user_val;
 	int err;
 	u8 regvalue;
 
-	/* Get userspace string and assure termination */
-	buf_size = min(count, (sizeof(buf)-1));
-	if (copy_from_user(buf, user_buf, buf_size))
-		return -EFAULT;
-	buf[buf_size] = 0;
-
-	err = strict_strtoul(buf, 0, &user_val);
+	/* Get userspace string and convert to number */
+	err = kstrtou8_from_user(user_buf, count, 0, &user_val);
 	if (err)
-		return -EINVAL;
-	if (user_val > 0xff) {
-		dev_err(&ab->i2c_client[0]->dev,
-			"debugfs error input > 0xff\n");
-		return -EINVAL;
-	}
+		return err;
+
 	err = mask_and_set_register_interruptible(
-		ab, (u8)ab->debug_bank,
-		(u8)ab->debug_address, 0xFF, (u8)user_val);
+		ab, ab->debug_bank,
+		ab->debug_address, 0xFF, user_val);
 	if (err)
 		return -EINVAL;
 
-	get_register_interruptible(ab, (u8)ab->debug_bank,
-		(u8)ab->debug_address, &regvalue);
+	get_register_interruptible(ab, ab->debug_bank,
+		ab->debug_address, &regvalue);
 	if (err)
 		return -EINVAL;
 
-	return buf_size;
+	return count;
 }
 
 static const struct file_operations ab3550_bank_fops = {
-- 
1.7.3.4


WARNING: multiple messages have this Message-ID (diff)
From: peterhuewe@gmx.de (Peter Huewe)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 1/2 v3] mfd/ab3550: Convert to kstrtou8_from_user
Date: Mon, 20 Jun 2011 23:01:37 +0200	[thread overview]
Message-ID: <1308603698-32166-1-git-send-email-peterhuewe@gmx.de> (raw)
In-Reply-To: <20110620195037.GA2534@p183.telecom.by>

This patch replaces the code for getting an number from a
userspace buffer by a simple call to kstrou8_from_user.
This makes it easier to read and less error prone.

Since the old buffers held only values up to 255, we don't need
kstrtoul, but rather kstrtou8.

Kernel Version: v3.0-rc3

Changes in v2:
- Use kstrtou8 instead of kstrtoul due to small numbers
- Dropped then unnecessary checks
Changes in v3:
- Drop now unnecessary local variables
- Change the debug_address and debug_bank members to u8 since they hold
  only values up to 255, and also remove the now superfluous u8 casts
(Remarks from Alexey Dobriyan, Thanks!)

Signed-off-by: Peter Huewe <peterhuewe@gmx.de>
---
 drivers/mfd/ab3550-core.c |   79 +++++++++++++-------------------------------
 1 files changed, 24 insertions(+), 55 deletions(-)

diff --git a/drivers/mfd/ab3550-core.c b/drivers/mfd/ab3550-core.c
index 3d7dce6..bd0df90 100644
--- a/drivers/mfd/ab3550-core.c
+++ b/drivers/mfd/ab3550-core.c
@@ -73,8 +73,8 @@ struct ab3550 {
 	u8 startup_events[AB3550_NUM_EVENT_REG];
 	bool startup_events_read;
 #ifdef CONFIG_DEBUG_FS
-	unsigned int debug_bank;
-	unsigned int debug_address;
+	u8 debug_bank;
+	u8 debug_address;
 #endif
 };
 
@@ -879,20 +879,13 @@ static ssize_t ab3550_bank_write(struct file *file,
 	size_t count, loff_t *ppos)
 {
 	struct ab3550 *ab = ((struct seq_file *)(file->private_data))->private;
-	char buf[32];
-	int buf_size;
-	unsigned long user_bank;
+	u8 user_bank;
 	int err;
 
-	/* Get userspace string and assure termination */
-	buf_size = min(count, (sizeof(buf) - 1));
-	if (copy_from_user(buf, user_buf, buf_size))
-		return -EFAULT;
-	buf[buf_size] = 0;
-
-	err = strict_strtoul(buf, 0, &user_bank);
+	/* Get userspace string and convert to number */
+	err = kstrtou8_from_user(user_buf, count, 0, &user_bank);
 	if (err)
-		return -EINVAL;
+		return err;
 
 	if (user_bank >= AB3550_NUM_BANKS) {
 		dev_err(&ab->i2c_client[0]->dev,
@@ -902,7 +895,7 @@ static ssize_t ab3550_bank_write(struct file *file,
 
 	ab->debug_bank = user_bank;
 
-	return buf_size;
+	return count;
 }
 
 static int ab3550_address_print(struct seq_file *s, void *p)
@@ -923,27 +916,14 @@ static ssize_t ab3550_address_write(struct file *file,
 	size_t count, loff_t *ppos)
 {
 	struct ab3550 *ab = ((struct seq_file *)(file->private_data))->private;
-	char buf[32];
-	int buf_size;
-	unsigned long user_address;
 	int err;
 
-	/* Get userspace string and assure termination */
-	buf_size = min(count, (sizeof(buf) - 1));
-	if (copy_from_user(buf, user_buf, buf_size))
-		return -EFAULT;
-	buf[buf_size] = 0;
-
-	err = strict_strtoul(buf, 0, &user_address);
+	/* Get userspace string and convert to number */
+	err = kstrtou8_from_user(user_buf, count, 0, &ab->debug_address);
 	if (err)
-		return -EINVAL;
-	if (user_address > 0xff) {
-		dev_err(&ab->i2c_client[0]->dev,
-			"debugfs error input > 0xff\n");
-		return -EINVAL;
-	}
-	ab->debug_address = user_address;
-	return buf_size;
+		return err;
+
+	return count;
 }
 
 static int ab3550_val_print(struct seq_file *s, void *p)
@@ -952,8 +932,8 @@ static int ab3550_val_print(struct seq_file *s, void *p)
 	int err;
 	u8 regvalue;
 
-	err = get_register_interruptible(ab, (u8)ab->debug_bank,
-		(u8)ab->debug_address, &regvalue);
+	err = get_register_interruptible(ab, ab->debug_bank,
+		ab->debug_address, &regvalue);
 	if (err)
 		return -EINVAL;
 	seq_printf(s, "0x%02X\n", regvalue);
@@ -971,38 +951,27 @@ static ssize_t ab3550_val_write(struct file *file,
 	size_t count, loff_t *ppos)
 {
 	struct ab3550 *ab = ((struct seq_file *)(file->private_data))->private;
-	char buf[32];
-	int buf_size;
-	unsigned long user_val;
+	u8 user_val;
 	int err;
 	u8 regvalue;
 
-	/* Get userspace string and assure termination */
-	buf_size = min(count, (sizeof(buf)-1));
-	if (copy_from_user(buf, user_buf, buf_size))
-		return -EFAULT;
-	buf[buf_size] = 0;
-
-	err = strict_strtoul(buf, 0, &user_val);
+	/* Get userspace string and convert to number */
+	err = kstrtou8_from_user(user_buf, count, 0, &user_val);
 	if (err)
-		return -EINVAL;
-	if (user_val > 0xff) {
-		dev_err(&ab->i2c_client[0]->dev,
-			"debugfs error input > 0xff\n");
-		return -EINVAL;
-	}
+		return err;
+
 	err = mask_and_set_register_interruptible(
-		ab, (u8)ab->debug_bank,
-		(u8)ab->debug_address, 0xFF, (u8)user_val);
+		ab, ab->debug_bank,
+		ab->debug_address, 0xFF, user_val);
 	if (err)
 		return -EINVAL;
 
-	get_register_interruptible(ab, (u8)ab->debug_bank,
-		(u8)ab->debug_address, &regvalue);
+	get_register_interruptible(ab, ab->debug_bank,
+		ab->debug_address, &regvalue);
 	if (err)
 		return -EINVAL;
 
-	return buf_size;
+	return count;
 }
 
 static const struct file_operations ab3550_bank_fops = {
-- 
1.7.3.4

  parent reply	other threads:[~2011-06-20 21:01 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-06-06 20:43 [PATCH 1/2] mfd/ab3550: Use kstrtoul_from_user Peter Huewe
2011-06-06 20:43 ` Peter Huewe
2011-06-06 20:43 ` Peter Huewe
2011-06-06 20:43 ` [PATCH 2/2] mfd/ab8500: " Peter Huewe
2011-06-06 20:43   ` Peter Huewe
2011-06-06 20:43   ` Peter Huewe
2011-06-20 13:45   ` Alexey Dobriyan
2011-06-20 13:45     ` Alexey Dobriyan
2011-06-20 13:45     ` Alexey Dobriyan
2011-06-20 19:46     ` Peter Hüwe
2011-06-20 19:46       ` Peter Hüwe
2011-06-20 19:46       ` Peter Hüwe
2011-06-20 19:46     ` [PATCH 1/2 v2] mfd/ab3550: Convert to kstrtou8_from_user Peter Huewe
2011-06-20 19:46       ` Peter Huewe
2011-06-20 19:46       ` Peter Huewe
2011-06-20 19:50       ` Alexey Dobriyan
2011-06-20 19:50         ` Alexey Dobriyan
2011-06-20 19:50         ` Alexey Dobriyan
2011-06-20 20:16         ` Peter Hüwe
2011-06-20 20:16           ` Peter Hüwe
2011-06-20 20:16           ` Peter Hüwe
2011-06-20 21:01         ` Peter Huewe [this message]
2011-06-20 21:01           ` [PATCH 1/2 v3] " Peter Huewe
2011-06-20 21:01           ` Peter Huewe
2011-06-20 21:01         ` [PATCH 2/2 v4] mfd/ab8500: " Peter Huewe
2011-06-20 21:01           ` Peter Huewe
2011-06-20 21:01           ` Peter Huewe
2011-06-20 19:46     ` [PATCH 2/2 v2] " Peter Huewe
2011-06-20 19:46       ` Peter Huewe
2011-06-20 19:46       ` Peter Huewe
2011-06-20 19:51       ` [PATCH v3] " Peter Huewe
2011-06-20 19:51         ` Peter Huewe
2011-06-20 19:51         ` Peter Huewe
2011-06-20 13:29 ` [PATCH 1/2] mfd/ab3550: Use kstrtoul_from_user Samuel Ortiz
2011-06-20 13:29   ` Samuel Ortiz
2011-06-20 13:29   ` Samuel Ortiz

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=1308603698-32166-1-git-send-email-peterhuewe@gmx.de \
    --to=peterhuewe@gmx.de \
    --cc=adobriyan@gmail.com \
    --cc=broonie@opensource.wolfsonmicro.com \
    --cc=dp@opensource.wolfsonmicro.com \
    --cc=ian@opensource.wolfsonmicro.com \
    --cc=kernel-janitors@vger.kernel.org \
    --cc=linus.walleij@stericsson.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=sameo@linux.intel.com \
    --cc=srinidhi.kasagar@stericsson.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 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.