[PATCH v4 00/12] introduce skip_spaces(), reducing code size plus some clean-ups

[PATCH v4 00/12] introduce skip_spaces(), reducing code size plus some clean-ups

[André Goddard Rosa]

This patch reduces lib.a code size by 173 bytes on my Core 2 with gcc 4.4.1
even considering that it exports a newly defined function skip_spaces()
to drivers:
   text    data     bss     dec     hex filename                           
  64867     840     592   66299   102fb (TOTALS-lib.a-before)
  64954     584     588   66126   1024e (TOTALS-lib.a-after)
and implements some code tidy up.

Besides reducing lib.a size, it converts many in-tree drivers to use the
newly defined function, which makes another small reduction on kernel size
overall when those drivers are used.

p.s.: Ingo, I hope to have finally fixed my environment by using send-email now.
Please let me know if you have any problem with these and thank you for the
good advice. get_maintainer.pl gave lots of people as output, I hope that's ok.

Changelog:
   v4: convert drivers over newly defined skip_spaces() function 
   v3: improved comments on patch 5/7 and factorize a switch statement on 6/7
as per suggestions from Ingo and Frederic (thanks!)
   v2: addressed feedback from Frederic Weisbecker review (thanks!!)
and split into separate patches
   v1: original submission

André Goddard Rosa (12):
  vsprintf: factorize "(null)" string
  vsprintf: pre-calculate final string length for later use
  vsprintf: give it some care to please checkpatch.pl
  vsprintf: use TOLOWER whenever possible
  vsprintf: reduce code size by avoiding extra check
  vsprintf: move local vars to block local vars and remove unneeded
    ones
  vsprintf: factor out skip_space code in a separate function
  vsprintf: reuse almost identical simple_strtoulX() functions
  ctype: constify read-only _ctype string
  string: factorize skip_spaces and export it to be generally available
  string: on strstrip(), first remove leading spaces before running
    over str
  tree-wide: convert open calls to remove spaces to skip_spaces() lib
    function

 arch/s390/kernel/debug.c              |    3 +-
 arch/um/drivers/mconsole_kern.c       |   16 +-
 arch/x86/kernel/cpu/mtrr/if.c         |   11 +-
 drivers/md/dm-table.c                 |    6 +-
 drivers/md/md.c                       |    4 +-
 drivers/parisc/pdc_stable.c           |    9 +-
 drivers/platform/x86/thinkpad_acpi.c  |    7 +-
 drivers/pnp/interface.c               |   36 +---
 drivers/s390/block/dasd_proc.c        |    5 +-
 drivers/video/backlight/lcd.c         |    4 +-
 drivers/video/display/display-sysfs.c |    2 +-
 fs/cachefiles/daemon.c                |    8 +-
 fs/ext4/super.c                       |    7 +-
 include/linux/ctype.h                 |    2 +-
 include/linux/string.h                |    1 +
 kernel/params.c                       |    8 +-
 lib/argv_split.c                      |   13 +-
 lib/ctype.c                           |   50 +++---
 lib/dynamic_debug.c                   |    4 +-
 lib/string.c                          |   19 ++-
 lib/vsprintf.c                        |  342 ++++++++++++++++-----------------
 net/irda/irnet/irnet.h                |    1 +
 net/irda/irnet/irnet_ppp.c            |    8 +-
 net/netfilter/xt_recent.c             |    3 +-
 sound/pci/hda/hda_hwdep.c             |    7 +-
 25 files changed, 264 insertions(+), 312 deletions(-)

[PATCH v4 10/12] string: factorize skip_spaces and export it to be generally available

[André Goddard Rosa]

On the following sentence:
    while (*s && isspace(*s))
        s++;

If *s == 0, isspace() evaluates to ((_ctype[*s] & 0x20) != 0), which
evaluates to ((0x08 & 0x20) != 0) which equals to 0 as well.
If *s == 1, we depend on isspace() result anyway.

In other words, "a char equals zero is never a space". So remove this check.
Also, *s != 0 is by far the most common case (non-empty string).

Signed-off-by: André Goddard Rosa <andre.goddard@gmail.com>
---
 include/linux/string.h |    1 +
 lib/string.c           |   19 +++++++++++++++----
 2 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/include/linux/string.h b/include/linux/string.h
index b850886..3bba9ee 100644
--- a/include/linux/string.h
+++ b/include/linux/string.h
@@ -62,6 +62,7 @@ extern char * strnchr(const char *, size_t, int);
 #ifndef __HAVE_ARCH_STRRCHR
 extern char * strrchr(const char *,int);
 #endif
+extern const char * __must_check skip_spaces(const char *);
 extern char * __must_check strstrip(char *);
 #ifndef __HAVE_ARCH_STRSTR
 extern char * strstr(const char *,const char *);
diff --git a/lib/string.c b/lib/string.c
index b19b87a..d9a51d5 100644
--- a/lib/string.c
+++ b/lib/string.c
@@ -330,6 +330,20 @@ EXPORT_SYMBOL(strnchr);
 #endif
 
 /**
+ * skip_spaces - Removes leading whitespace from @s.
+ * @s: The string to be stripped.
+ *
+ * Returns a pointer to the first non-whitespace character in @s.
+ */
+const char *skip_spaces(const char *str)
+{
+	while (isspace(*str))
+		++str;
+	return str;
+}
+EXPORT_SYMBOL(skip_spaces);
+
+/**
  * strstrip - Removes leading and trailing whitespace from @s.
  * @s: The string to be stripped.
  *
@@ -352,10 +366,7 @@ char *strstrip(char *s)
 		end--;
 	*(end + 1) = '\0';
 
-	while (*s && isspace(*s))
-		s++;
-
-	return s;
+	return (char *)skip_spaces(s);
 }
 EXPORT_SYMBOL(strstrip);
 
-- 
1.6.5.2.153.g6e31f.dirty

[PATCH v4 12/12] tree-wide: convert open calls to remove spaces to skip_spaces() lib function

[André Goddard Rosa]

Makes use of skip_spaces() defined in lib/string.c for removing leading
spaces from strings all over the tree.

Also, while at it, if we see (*str && isspace(*str)), we can be sure to
remove the first condition (*str) as the second one (isspace(*str)) also
evaluates to 0 whenever *str == 0, making it redundant. In other words,
"a char equals zero is never a space".

Signed-off-by: André Goddard Rosa <andre.goddard@gmail.com>
---
 arch/s390/kernel/debug.c              |    3 +-
 arch/um/drivers/mconsole_kern.c       |   16 ++++++--------
 arch/x86/kernel/cpu/mtrr/if.c         |   11 +++------
 drivers/md/dm-table.c                 |    6 +---
 drivers/md/md.c                       |    4 +-
 drivers/parisc/pdc_stable.c           |    9 ++-----
 drivers/platform/x86/thinkpad_acpi.c  |    7 +----
 drivers/pnp/interface.c               |   36 +++++++++-----------------------
 drivers/s390/block/dasd_proc.c        |    5 ++-
 drivers/video/backlight/lcd.c         |    4 +-
 drivers/video/display/display-sysfs.c |    2 +-
 fs/cachefiles/daemon.c                |    8 ++----
 fs/ext4/super.c                       |    7 +----
 kernel/params.c                       |    8 ++----
 lib/argv_split.c                      |   13 ++---------
 lib/dynamic_debug.c                   |    4 +-
 lib/vsprintf.c                        |   15 +++----------
 net/irda/irnet/irnet.h                |    1 +
 net/irda/irnet/irnet_ppp.c            |    8 ++----
 net/netfilter/xt_recent.c             |    3 +-
 sound/pci/hda/hda_hwdep.c             |    7 ++---
 21 files changed, 63 insertions(+), 114 deletions(-)

diff --git a/arch/s390/kernel/debug.c b/arch/s390/kernel/debug.c
index 20f282c..7a0f4a9 100644
--- a/arch/s390/kernel/debug.c
+++ b/arch/s390/kernel/debug.c
@@ -18,6 +18,7 @@
 #include <linux/errno.h>
 #include <linux/slab.h>
 #include <linux/ctype.h>
+#include <linux/string.h>
 #include <linux/sysctl.h>
 #include <asm/uaccess.h>
 #include <linux/module.h>
@@ -1183,7 +1184,7 @@ debug_get_uint(char *buf)
 {
 	int rc;
 
-	for(; isspace(*buf); buf++);
+	buf = skip_spaces(buf);
 	rc = simple_strtoul(buf, &buf, 10);
 	if(*buf){
 		rc = -EINVAL;
diff --git a/arch/um/drivers/mconsole_kern.c b/arch/um/drivers/mconsole_kern.c
index e14629c..f5d459a 100644
--- a/arch/um/drivers/mconsole_kern.c
+++ b/arch/um/drivers/mconsole_kern.c
@@ -6,6 +6,7 @@
 
 #include <linux/console.h>
 #include <linux/ctype.h>
+#include <linux/string.h>
 #include <linux/interrupt.h>
 #include <linux/list.h>
 #include <linux/mm.h>
@@ -131,7 +132,7 @@ void mconsole_proc(struct mc_request *req)
 	char *ptr = req->request.data, *buf;
 
 	ptr += strlen("proc");
-	while (isspace(*ptr)) ptr++;
+	ptr = skip_spaces(ptr);
 
 	proc = get_fs_type("proc");
 	if (proc == NULL) {
@@ -212,8 +213,7 @@ void mconsole_proc(struct mc_request *req)
 	char *ptr = req->request.data;
 
 	ptr += strlen("proc");
-	while (isspace(*ptr))
-		ptr++;
+	ptr = skip_spaces(ptr);
 	snprintf(path, sizeof(path), "/proc/%s", ptr);
 
 	fd = sys_open(path, 0, 0);
@@ -560,8 +560,7 @@ void mconsole_config(struct mc_request *req)
 	int err;
 
 	ptr += strlen("config");
-	while (isspace(*ptr))
-		ptr++;
+	ptr = skip_spaces(ptr);
 	dev = mconsole_find_dev(ptr);
 	if (dev == NULL) {
 		mconsole_reply(req, "Bad configuration option", 1, 0);
@@ -588,7 +587,7 @@ void mconsole_remove(struct mc_request *req)
 	int err, start, end, n;
 
 	ptr += strlen("remove");
-	while (isspace(*ptr)) ptr++;
+	ptr = skip_spaces(ptr);
 	dev = mconsole_find_dev(ptr);
 	if (dev == NULL) {
 		mconsole_reply(req, "Bad remove option", 1, 0);
@@ -712,7 +711,7 @@ void mconsole_sysrq(struct mc_request *req)
 	char *ptr = req->request.data;
 
 	ptr += strlen("sysrq");
-	while (isspace(*ptr)) ptr++;
+	ptr = skip_spaces(ptr);
 
 	/*
 	 * With 'b', the system will shut down without a chance to reply,
@@ -757,8 +756,7 @@ void mconsole_stack(struct mc_request *req)
 	 */
 
 	ptr += strlen("stack");
-	while (isspace(*ptr))
-		ptr++;
+	ptr = skip_spaces(ptr);
 
 	/*
 	 * Should really check for multiple pids or reject bad args here
diff --git a/arch/x86/kernel/cpu/mtrr/if.c b/arch/x86/kernel/cpu/mtrr/if.c
index 3c1b12d..e006e56 100644
--- a/arch/x86/kernel/cpu/mtrr/if.c
+++ b/arch/x86/kernel/cpu/mtrr/if.c
@@ -4,6 +4,7 @@
 #include <linux/proc_fs.h>
 #include <linux/module.h>
 #include <linux/ctype.h>
+#include <linux/string.h>
 #include <linux/init.h>
 
 #define LINE_SIZE 80
@@ -133,8 +134,7 @@ mtrr_write(struct file *file, const char __user *buf, size_t len, loff_t * ppos)
 		return -EINVAL;
 
 	base = simple_strtoull(line + 5, &ptr, 0);
-	while (isspace(*ptr))
-		ptr++;
+	ptr = skip_spaces(ptr);
 
 	if (strncmp(ptr, "size=", 5))
 		return -EINVAL;
@@ -142,14 +142,11 @@ mtrr_write(struct file *file, const char __user *buf, size_t len, loff_t * ppos)
 	size = simple_strtoull(ptr + 5, &ptr, 0);
 	if ((base & 0xfff) || (size & 0xfff))
 		return -EINVAL;
-	while (isspace(*ptr))
-		ptr++;
+	ptr = skip_spaces(ptr);
 
 	if (strncmp(ptr, "type=", 5))
 		return -EINVAL;
-	ptr += 5;
-	while (isspace(*ptr))
-		ptr++;
+	ptr = skip_spaces(ptr + 5);
 
 	for (i = 0; i < MTRR_NUM_TYPES; ++i) {
 		if (strcmp(ptr, mtrr_strings[i]))
diff --git a/drivers/md/dm-table.c b/drivers/md/dm-table.c
index 1a6cb3c..91976e8 100644
--- a/drivers/md/dm-table.c
+++ b/drivers/md/dm-table.c
@@ -12,6 +12,7 @@
 #include <linux/blkdev.h>
 #include <linux/namei.h>
 #include <linux/ctype.h>
+#include <linux/string.h>
 #include <linux/slab.h>
 #include <linux/interrupt.h>
 #include <linux/mutex.h>
@@ -600,11 +601,8 @@ int dm_split_args(int *argc, char ***argvp, char *input)
 		return -ENOMEM;
 
 	while (1) {
-		start = end;
-
 		/* Skip whitespace */
-		while (*start && isspace(*start))
-			start++;
+		start = skip_spaces(end);
 
 		if (!*start)
 			break;	/* success, we hit the end */
diff --git a/drivers/md/md.c b/drivers/md/md.c
index 26ba42a..1dcb83c 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -39,6 +39,7 @@
 #include <linux/buffer_head.h> /* for invalidate_bdev */
 #include <linux/poll.h>
 #include <linux/ctype.h>
+#include <linux/string.h>
 #include <linux/hdreg.h>
 #include <linux/proc_fs.h>
 #include <linux/random.h>
@@ -3225,8 +3226,7 @@ bitmap_store(mddev_t *mddev, const char *buf, size_t len)
 		}
 		if (*end && !isspace(*end)) break;
 		bitmap_dirty_bits(mddev->bitmap, chunk, end_chunk);
-		buf = end;
-		while (isspace(*buf)) buf++;
+		buf = skip_spaces(end);
 	}
 	bitmap_unplug(mddev->bitmap); /* flush the bits to disk */
 out:
diff --git a/drivers/parisc/pdc_stable.c b/drivers/parisc/pdc_stable.c
index 13a64bc..0bc5d47 100644
--- a/drivers/parisc/pdc_stable.c
+++ b/drivers/parisc/pdc_stable.c
@@ -779,12 +779,9 @@ static ssize_t pdcs_auto_write(struct kobject *kobj,
 	read_unlock(&pathentry->rw_lock);
 	
 	DPRINTK("%s: flags before: 0x%X\n", __func__, flags);
-			
-	temp = in;
-	
-	while (*temp && isspace(*temp))
-		temp++;
-	
+
+	temp = skip_spaces(in);
+
 	c = *temp++ - '0';
 	if ((c != 0) && (c != 1))
 		goto parse_error;
diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
index 7e7da18..29dd2dd 100644
--- a/drivers/platform/x86/thinkpad_acpi.c
+++ b/drivers/platform/x86/thinkpad_acpi.c
@@ -1006,11 +1006,8 @@ static int parse_strtoul(const char *buf,
 {
 	char *endp;
 
-	while (*buf && isspace(*buf))
-		buf++;
-	*value = simple_strtoul(buf, &endp, 0);
-	while (*endp && isspace(*endp))
-		endp++;
+	*value = simple_strtoul(skip_spaces(buf), &endp, 0);
+	endp = skip_spaces(endp);
 	if (*endp || *value > max)
 		return -EINVAL;
 
diff --git a/drivers/pnp/interface.c b/drivers/pnp/interface.c
index c3f1c8e..68b0c04 100644
--- a/drivers/pnp/interface.c
+++ b/drivers/pnp/interface.c
@@ -310,8 +310,7 @@ static ssize_t pnp_set_current_resources(struct device *dmdev,
 		goto done;
 	}
 
-	while (isspace(*buf))
-		++buf;
+	buf = skip_spaces(buf);
 	if (!strnicmp(buf, "disable", 7)) {
 		retval = pnp_disable_dev(dev);
 		goto done;
@@ -353,19 +352,13 @@ static ssize_t pnp_set_current_resources(struct device *dmdev,
 		pnp_init_resources(dev);
 		mutex_lock(&pnp_res_mutex);
 		while (1) {
-			while (isspace(*buf))
-				++buf;
+			buf = skip_spaces(buf);
 			if (!strnicmp(buf, "io", 2)) {
-				buf += 2;
-				while (isspace(*buf))
-					++buf;
+				buf = skip_spaces(buf + 2);
 				start = simple_strtoul(buf, &buf, 0);
-				while (isspace(*buf))
-					++buf;
+				buf = skip_spaces(buf);
 				if (*buf == '-') {
-					buf += 1;
-					while (isspace(*buf))
-						++buf;
+					buf = skip_spaces(buf + 1);
 					end = simple_strtoul(buf, &buf, 0);
 				} else
 					end = start;
@@ -373,16 +366,11 @@ static ssize_t pnp_set_current_resources(struct device *dmdev,
 				continue;
 			}
 			if (!strnicmp(buf, "mem", 3)) {
-				buf += 3;
-				while (isspace(*buf))
-					++buf;
+				buf = skip_spaces(buf + 3);
 				start = simple_strtoul(buf, &buf, 0);
-				while (isspace(*buf))
-					++buf;
+				buf = skip_spaces(buf);
 				if (*buf == '-') {
-					buf += 1;
-					while (isspace(*buf))
-						++buf;
+					buf = skip_spaces(buf + 1);
 					end = simple_strtoul(buf, &buf, 0);
 				} else
 					end = start;
@@ -390,17 +378,13 @@ static ssize_t pnp_set_current_resources(struct device *dmdev,
 				continue;
 			}
 			if (!strnicmp(buf, "irq", 3)) {
-				buf += 3;
-				while (isspace(*buf))
-					++buf;
+				buf = skip_spaces(buf + 3);
 				start = simple_strtoul(buf, &buf, 0);
 				pnp_add_irq_resource(dev, start, 0);
 				continue;
 			}
 			if (!strnicmp(buf, "dma", 3)) {
-				buf += 3;
-				while (isspace(*buf))
-					++buf;
+				buf = skip_spaces(buf + 3);
 				start = simple_strtoul(buf, &buf, 0);
 				pnp_add_dma_resource(dev, start, 0);
 				continue;
diff --git a/drivers/s390/block/dasd_proc.c b/drivers/s390/block/dasd_proc.c
index 654daa3..c2d8792 100644
--- a/drivers/s390/block/dasd_proc.c
+++ b/drivers/s390/block/dasd_proc.c
@@ -14,6 +14,7 @@
 #define KMSG_COMPONENT "dasd"
 
 #include <linux/ctype.h>
+#include <linux/string.h>
 #include <linux/seq_file.h>
 #include <linux/vmalloc.h>
 #include <linux/proc_fs.h>
@@ -272,10 +273,10 @@ dasd_statistics_write(struct file *file, const char __user *user_buf,
 	DBF_EVENT(DBF_DEBUG, "/proc/dasd/statictics: '%s'\n", buffer);
 
 	/* check for valid verbs */
-	for (str = buffer; isspace(*str); str++);
+	str = skip_spaces(buffer);
 	if (strncmp(str, "set", 3) == 0 && isspace(str[3])) {
 		/* 'set xxx' was given */
-		for (str = str + 4; isspace(*str); str++);
+		str = skip_spaces(str + 4);
 		if (strcmp(str, "on") == 0) {
 			/* switch on statistics profiling */
 			dasd_profile_level = DASD_PROFILE_ON;
diff --git a/drivers/video/backlight/lcd.c b/drivers/video/backlight/lcd.c
index b644947..3b20cbf 100644
--- a/drivers/video/backlight/lcd.c
+++ b/drivers/video/backlight/lcd.c
@@ -101,7 +101,7 @@ static ssize_t lcd_store_power(struct device *dev,
 	int power = simple_strtoul(buf, &endp, 0);
 	size_t size = endp - buf;
 
-	if (*endp && isspace(*endp))
+	if (isspace(*endp))
 		size++;
 	if (size != count)
 		return -EINVAL;
@@ -140,7 +140,7 @@ static ssize_t lcd_store_contrast(struct device *dev,
 	int contrast = simple_strtoul(buf, &endp, 0);
 	size_t size = endp - buf;
 
-	if (*endp && isspace(*endp))
+	if (isspace(*endp))
 		size++;
 	if (size != count)
 		return -EINVAL;
diff --git a/drivers/video/display/display-sysfs.c b/drivers/video/display/display-sysfs.c
index 4830b1b..80abbf3 100644
--- a/drivers/video/display/display-sysfs.c
+++ b/drivers/video/display/display-sysfs.c
@@ -67,7 +67,7 @@ static ssize_t display_store_contrast(struct device *dev,
 	contrast = simple_strtoul(buf, &endp, 0);
 	size = endp - buf;
 
-	if (*endp && isspace(*endp))
+	if (isspace(*endp))
 		size++;
 
 	if (size != count)
diff --git a/fs/cachefiles/daemon.c b/fs/cachefiles/daemon.c
index 4618516..af6d9a6 100644
--- a/fs/cachefiles/daemon.c
+++ b/fs/cachefiles/daemon.c
@@ -21,6 +21,7 @@
 #include <linux/mount.h>
 #include <linux/statfs.h>
 #include <linux/ctype.h>
+#include <linux/string.h>
 #include <linux/fs_struct.h>
 #include "internal.h"
 
@@ -250,15 +251,12 @@ static ssize_t cachefiles_daemon_write(struct file *file,
 	/* parse the command */
 	ret = -EOPNOTSUPP;
 
-	for (args = data; *args; args++)
-		if (isspace(*args))
-			break;
+	args = skip_spaces(data);
 	if (*args) {
 		if (args == data)
 			goto error;
 		*args = '\0';
-		for (args++; isspace(*args); args++)
-			continue;
+		args = skip_spaces(++args);
 	}
 
 	/* run the appropriate command handler */
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 312211e..580766a 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -2097,11 +2097,8 @@ static int parse_strtoul(const char *buf,
 {
 	char *endp;
 
-	while (*buf && isspace(*buf))
-		buf++;
-	*value = simple_strtoul(buf, &endp, 0);
-	while (*endp && isspace(*endp))
-		endp++;
+	*value = simple_strtoul(skip_spaces(buf), &endp, 0);
+	endp = skip_spaces(endp);
 	if (*endp || *value > max)
 		return -EINVAL;
 
diff --git a/kernel/params.c b/kernel/params.c
index d656c27..cf1b691 100644
--- a/kernel/params.c
+++ b/kernel/params.c
@@ -24,6 +24,7 @@
 #include <linux/err.h>
 #include <linux/slab.h>
 #include <linux/ctype.h>
+#include <linux/string.h>
 
 #if 0
 #define DEBUGP printk
@@ -122,9 +123,7 @@ static char *next_arg(char *args, char **param, char **val)
 		next = args + i;
 
 	/* Chew up trailing spaces. */
-	while (isspace(*next))
-		next++;
-	return next;
+	return skip_spaces(next);
 }
 
 /* Args looks like "foo=bar,bar2 baz=fuz wiz". */
@@ -139,8 +138,7 @@ int parse_args(const char *name,
 	DEBUGP("Parsing ARGS: %s\n", args);
 
 	/* Chew leading spaces */
-	while (isspace(*args))
-		args++;
+	args = skip_spaces(args);
 
 	while (*args) {
 		int ret;
diff --git a/lib/argv_split.c b/lib/argv_split.c
index 5205a8d..4b1b083 100644
--- a/lib/argv_split.c
+++ b/lib/argv_split.c
@@ -4,17 +4,10 @@
 
 #include <linux/kernel.h>
 #include <linux/ctype.h>
+#include <linux/string.h>
 #include <linux/slab.h>
 #include <linux/module.h>
 
-static const char *skip_sep(const char *cp)
-{
-	while (*cp && isspace(*cp))
-		cp++;
-
-	return cp;
-}
-
 static const char *skip_arg(const char *cp)
 {
 	while (*cp && !isspace(*cp))
@@ -28,7 +21,7 @@ static int count_argc(const char *str)
 	int count = 0;
 
 	while (*str) {
-		str = skip_sep(str);
+		str = skip_spaces(str);
 		if (*str) {
 			count++;
 			str = skip_arg(str);
@@ -82,7 +75,7 @@ char **argv_split(gfp_t gfp, const char *str, int *argcp)
 	argvp = argv;
 
 	while (*str) {
-		str = skip_sep(str);
+		str = skip_spaces(str);
 
 		if (*str) {
 			const char *p = str;
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index e22c148..f935029 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -21,6 +21,7 @@
 #include <linux/list.h>
 #include <linux/sysctl.h>
 #include <linux/ctype.h>
+#include <linux/string.h>
 #include <linux/uaccess.h>
 #include <linux/dynamic_debug.h>
 #include <linux/debugfs.h>
@@ -209,8 +210,7 @@ static int ddebug_tokenize(char *buf, char *words[], int maxwords)
 		char *end;
 
 		/* Skip leading whitespace */
-		while (*buf && isspace(*buf))
-			buf++;
+		buf = skip_spaces(buf);
 		if (!*buf)
 			break;	/* oh, it was trailing whitespace */
 
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 7ec96a3..06e0311 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -1716,13 +1716,6 @@ EXPORT_SYMBOL_GPL(bprintf);
 
 #endif /* CONFIG_BINARY_PRINTF */
 
-static noinline const char *skip_space(const char *str)
-{
-	while (isspace(*str))
-		++str;
-	return str;
-}
-
 /**
  * vsscanf - Unformat a buffer into a list of arguments
  * @buf:	input buffer
@@ -1744,8 +1737,8 @@ int vsscanf(const char *buf, const char *fmt, va_list args)
 		 * white space, including none, in the input.
 		 */
 		if (isspace(*fmt)) {
-			fmt = skip_space(fmt);
-			str = skip_space(str);
+			fmt = skip_spaces(++fmt);
+			str = skip_spaces(str);
 		}
 
 		/* anything that is not a conversion must match exactly */
@@ -1815,7 +1808,7 @@ int vsscanf(const char *buf, const char *fmt, va_list args)
 			if (field_width == -1)
 				field_width = INT_MAX;
 			/* first, skip leading white space in buffer */
-			str = skip_space(str);
+			str = skip_spaces(str);
 
 			/* now copy until next white space */
 			while (*str && !isspace(*str) && field_width--)
@@ -1857,7 +1850,7 @@ int vsscanf(const char *buf, const char *fmt, va_list args)
 		/* have some sort of integer conversion.
 		 * first, skip white space in buffer.
 		 */
-		str = skip_space(str);
+		str = skip_spaces(str);
 
 		digit = *str;
 		if (is_sign && digit == '-')
diff --git a/net/irda/irnet/irnet.h b/net/irda/irnet/irnet.h
index b001c36..4300df3 100644
--- a/net/irda/irnet/irnet.h
+++ b/net/irda/irnet/irnet.h
@@ -249,6 +249,7 @@
 #include <linux/poll.h>
 #include <linux/capability.h>
 #include <linux/ctype.h>	/* isspace() */
+#include <linux/string.h>	/* skip_spaces() */
 #include <asm/uaccess.h>
 #include <linux/init.h>
 
diff --git a/net/irda/irnet/irnet_ppp.c b/net/irda/irnet/irnet_ppp.c
index 7dea882..156020d 100644
--- a/net/irda/irnet/irnet_ppp.c
+++ b/net/irda/irnet/irnet_ppp.c
@@ -76,9 +76,8 @@ irnet_ctrl_write(irnet_socket *	ap,
       /* Look at the next command */
       start = next;
 
-      /* Scrap whitespaces before the command */
-      while(isspace(*start))
-	start++;
+	/* Scrap whitespaces before the command */
+	start = skip_spaces(start);
 
       /* ',' is our command separator */
       next = strchr(start, ',');
@@ -133,8 +132,7 @@ irnet_ctrl_write(irnet_socket *	ap,
 	      char *	endp;
 
 	      /* Scrap whitespaces before the command */
-	      while(isspace(*begp))
-		begp++;
+	      begp = skip_spaces(begp);
 
 	      /* Convert argument to a number (last arg is the base) */
 	      addr = simple_strtoul(begp, &endp, 16);
diff --git a/net/netfilter/xt_recent.c b/net/netfilter/xt_recent.c
index eb0ceb8..fc70a49 100644
--- a/net/netfilter/xt_recent.c
+++ b/net/netfilter/xt_recent.c
@@ -482,8 +482,7 @@ static ssize_t recent_old_proc_write(struct file *file,
 	if (copy_from_user(buf, input, size))
 		return -EFAULT;
 
-	while (isspace(*c))
-		c++;
+	c = skip_spaces(c);
 
 	if (size - (c - buf) < 5)
 		return c - buf;
diff --git a/sound/pci/hda/hda_hwdep.c b/sound/pci/hda/hda_hwdep.c
index cc24e67..4e17b43 100644
--- a/sound/pci/hda/hda_hwdep.c
+++ b/sound/pci/hda/hda_hwdep.c
@@ -24,6 +24,7 @@
 #include <linux/compat.h>
 #include <linux/mutex.h>
 #include <linux/ctype.h>
+#include <linux/string.h>
 #include <linux/firmware.h>
 #include <sound/core.h>
 #include "hda_codec.h"
@@ -390,8 +391,7 @@ static int parse_hints(struct hda_codec *codec, const char *buf)
 	char *key, *val;
 	struct hda_hint *hint;
 
-	while (isspace(*buf))
-		buf++;
+	buf = skip_spaces(buf);
 	if (!*buf || *buf == '#' || *buf == '\n')
 		return 0;
 	if (*buf == '=')
@@ -406,8 +406,7 @@ static int parse_hints(struct hda_codec *codec, const char *buf)
 		return -EINVAL;
 	}
 	*val++ = 0;
-	while (isspace(*val))
-		val++;
+	val = skip_spaces(val);
 	remove_trail_spaces(key);
 	remove_trail_spaces(val);
 	hint = get_hint(codec, key);
-- 
1.6.5.2.153.g6e31f.dirty

Re: [dm-devel] [PATCH v4 00/12] introduce skip_spaces(), reducing code size plus some clean-ups

[James Bottomley]

On Sat, 2009-11-07 at 13:16 -0200, André Goddard Rosa wrote:
> This patch reduces lib.a code size by 173 bytes on my Core 2 with gcc 4.4.1
> even considering that it exports a newly defined function skip_spaces()
> to drivers:
>    text    data     bss     dec     hex filename                           
>   64867     840     592   66299   102fb (TOTALS-lib.a-before)
>   64954     584     588   66126   1024e (TOTALS-lib.a-after)
> and implements some code tidy up.
> 
> Besides reducing lib.a size, it converts many in-tree drivers to use the
> newly defined function, which makes another small reduction on kernel size
> overall when those drivers are used.

Before we embark on something as massive as this, could we take a step
back.  I agree that if I were coming up with the strstip() interface
today I probably wouldn't have given it two overloaded uses.

However, I think the function, in spite of this minor issue, is very
usable.  I still don't understand why people thought adding a
__must_check, which is what damaged one of the overloaded uses, is a
good idea.

Assuming there's a good answer to the above:

> + * skip_spaces - Removes leading whitespace from @s.
> + * @s: The string to be stripped.
> + *
> + * Returns a pointer to the first non-whitespace character in @s.
> + */
> +const char *skip_spaces(const char *str)

I don't think const return is a good idea because most functions will be
manipulating the string and using pointers that won't be const, so this
will generate a ton of 'initialization discards qualifiers from pointer
target type' ... so that leads to the question of whether this patch
series was actually compiled ...

James

Re: [PATCH v4 10/12] string: factorize skip_spaces and export it to be generally available

[Alan Cox]

On Sat,  7 Nov 2009 13:16:18 -0200
André Goddard Rosa <andre.goddard@gmail.com> wrote:

> On the following sentence:
>     while (*s && isspace(*s))
>         s++;

Looks fine but for one thing: it's actually shorter inline than moved
into /lib so at the very least it should be a header inline not a
function call.

Second minor comment. Although it never made it into the final ANSI C,
the proposed name (and the one used in a lot of other non Linux code for
this) is stpblk().

Alan

Re: [dm-devel] [PATCH v4 00/12] introduce skip_spaces(), reducing code size plus some clean-ups

[André Goddard Rosa]

Hi, James!

On Sun, Nov 8, 2009 at 2:05 PM, James Bottomley
<James.Bottomley@hansenpartnership.com> wrote:
> On Sat, 2009-11-07 at 13:16 -0200, André Goddard Rosa wrote:
>> This patch reduces lib.a code size by 173 bytes on my Core 2 with gcc 4.4.1
>> even considering that it exports a newly defined function skip_spaces()
>> to drivers:
>>    text    data     bss     dec     hex filename
>>   64867     840     592   66299   102fb (TOTALS-lib.a-before)
>>   64954     584     588   66126   1024e (TOTALS-lib.a-after)
>> and implements some code tidy up.
>>
>> Besides reducing lib.a size, it converts many in-tree drivers to use the
>> newly defined function, which makes another small reduction on kernel size
>> overall when those drivers are used.
>
> Before we embark on something as massive as this, could we take a step
> back.  I agree that if I were coming up with the strstip() interface
> today I probably wouldn't have given it two overloaded uses.
>
> However, I think the function, in spite of this minor issue, is very
> usable.  I still don't understand why people thought adding a
> __must_check, which is what damaged one of the overloaded uses, is a
> good idea.

Differently of "static void strip(char *str)"@scripts/kconfig/conf.c ,
this function
does not moves the characters to the beginning of the string, so that if that
string is going to be reused it should refer to the newly returned string start.

I've changed it to remove the const and return a "char *".

Do you think __must_check is not needed as well?

Thanks,
André

Re: [PATCH v4 12/12] tree-wide: convert open calls to remove spaces to skip_spaces() lib function

[Theodore Tso]

On Sat, Nov 07, 2009 at 01:16:20PM -0200, André Goddard Rosa wrote:
> Makes use of skip_spaces() defined in lib/string.c for removing leading
> spaces from strings all over the tree.
> 
> Also, while at it, if we see (*str && isspace(*str)), we can be sure to
> remove the first condition (*str) as the second one (isspace(*str)) also
> evaluates to 0 whenever *str == 0, making it redundant. In other words,
> "a char equals zero is never a space".

There are a number of places that have the pattern of skipping
whitespace, calling simpler_strtoul(), and then skipping whitespace
afterwards.  And thinkpad_acpi.c and fs/ext4/super.c both have an
indentical function, parse_strotul(), which basically does this plus
doing actual error checking (a number of callers of simple_strtoul
aren't checking to see if the user passed in a valid number or not,
boo.)

I would suggest that we should lift parse_strtoul() into lib/, both to
save a bit of code, as well as encouraging people to do proper input
validation, while we are doing this tree-wide cleanup.

						- Ted

Re: [PATCH v4 12/12] tree-wide: convert open calls to remove spaces to skip_spaces() lib function

[Julia Lawall]

> > Also, while at it, if we see (*str && isspace(*str)), we can be sure to
> > remove the first condition (*str) as the second one (isspace(*str)) also
> > evaluates to 0 whenever *str == 0, making it redundant. In other words,
> > "a char equals zero is never a space".

I tried the following semantic patch (http://coccinelle.lip6.fr), and got 
the results below.

@@
expression str;
@@

( // ignore skip_spaces cases
 while (*str &&  isspace(*str)) { \(str++;\|++str;\) }
|
- *str && 
  isspace(*str)
)

I haven't checked the results in any way, however.

julia

diff -u -p a/drivers/leds/led-class.c b/drivers/leds/led-class.c
--- a/drivers/leds/led-class.c
+++ b/drivers/leds/led-class.c
@@ -50,7 +50,7 @@ static ssize_t led_brightness_store(stru
 	unsigned long state = simple_strtoul(buf, &after, 10);
 	size_t count = after - buf;
 
-	if (*after && isspace(*after))
+	if (isspace(*after))
 		count++;
 
 	if (count == size) {
diff -u -p a/drivers/leds/ledtrig-timer.c b/drivers/leds/ledtrig-timer.c
--- a/drivers/leds/ledtrig-timer.c
+++ b/drivers/leds/ledtrig-timer.c
@@ -83,7 +83,7 @@ static ssize_t led_delay_on_store(struct
 	unsigned long state = simple_strtoul(buf, &after, 10);
 	size_t count = after - buf;
 
-	if (*after && isspace(*after))
+	if (isspace(*after))
 		count++;
 
 	if (count == size) {
@@ -127,7 +127,7 @@ static ssize_t led_delay_off_store(struc
 	unsigned long state = simple_strtoul(buf, &after, 10);
 	size_t count = after - buf;
 
-	if (*after && isspace(*after))
+	if (isspace(*after))
 		count++;
 
 	if (count == size) {
diff -u -p a/drivers/video/backlight/lcd.c b/drivers/video/backlight/lcd.c
--- a/drivers/video/backlight/lcd.c
+++ b/drivers/video/backlight/lcd.c
@@ -101,7 +101,7 @@ static ssize_t lcd_store_power(struct de
 	int power = simple_strtoul(buf, &endp, 0);
 	size_t size = endp - buf;
 
-	if (*endp && isspace(*endp))
+	if (isspace(*endp))
 		size++;
 	if (size != count)
 		return -EINVAL;
@@ -140,7 +140,7 @@ static ssize_t lcd_store_contrast(struct
 	int contrast = simple_strtoul(buf, &endp, 0);
 	size_t size = endp - buf;
 
-	if (*endp && isspace(*endp))
+	if (isspace(*endp))
 		size++;
 	if (size != count)
 		return -EINVAL;
diff -u -p a/drivers/video/display/display-sysfs.c b/drivers/video/display/display-sysfs.c
--- a/drivers/video/display/display-sysfs.c
+++ b/drivers/video/display/display-sysfs.c
@@ -67,7 +67,7 @@ static ssize_t display_store_contrast(st
 	contrast = simple_strtoul(buf, &endp, 0);
 	size = endp - buf;
 
-	if (*endp && isspace(*endp))
+	if (isspace(*endp))
 		size++;
 
 	if (size != count)
diff -u -p a/drivers/video/output.c b/drivers/video/output.c
--- a/drivers/video/output.c
+++ b/drivers/video/output.c
@@ -50,7 +50,7 @@ static ssize_t video_output_store_state(
 	int request_state = simple_strtoul(buf,&endp,0);
 	size_t size = endp - buf;
 
-	if (*endp && isspace(*endp))
+	if (isspace(*endp))
 		size++;
 	if (size != count)
 		return -EINVAL;