This patch reduces lib/lib.a code size by 482 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)
64641 584 592 65817 10119 (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.
Changelog:
v5: addressed comments and feedback from many people: Jan Engelhardt,
James Bottomley, Rusty Russell, Alan Cox, Henrique de Moraes Holschuh,
Theodore Tso and Julia Lawall (thanks!)
v4: converted 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/leds/led-class.c | 2 +-
drivers/leds/ledtrig-timer.c | 4 +-
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 +-
drivers/video/output.c | 2 +-
fs/cachefiles/daemon.c | 4 +-
fs/ext4/super.c | 7 +-
include/linux/ctype.h | 3 +-
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 | 337 ++++++++++++++++-----------------
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 +-
28 files changed, 264 insertions(+), 312 deletions(-)
Change "<NULL>" to "(null)", unifying 3 equal strings.
glibc also uses "(null)" for the same purpose.
It decreases code size by 7 bytes:
text data bss dec hex filename
15765 0 8 15773 3d9d vsprintf.o (ex lib/lib.a-BEFORE)
15758 0 8 15766 3d96 vsprintf.o (ex lib/lib.a-AFTER)
Signed-off-by: André Goddard Rosa <[email protected]>
Acked-by: Frederic Weisbecker <[email protected]>
---
lib/vsprintf.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 33bed5e..fe56825 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -546,12 +546,12 @@ static char *number(char *buf, char *end, unsigned long long num,
return buf;
}
-static char *string(char *buf, char *end, char *s, struct printf_spec spec)
+static char *string(char *buf, char *end, const char *s, struct printf_spec spec)
{
int len, i;
if ((unsigned long)s < PAGE_SIZE)
- s = "<NULL>";
+ s = "(null)";
len = strnlen(s, spec.precision);
@@ -1445,7 +1445,7 @@ do { \
size_t len;
if ((unsigned long)save_str > (unsigned long)-PAGE_SIZE
|| (unsigned long)save_str < PAGE_SIZE)
- save_str = "<NULL>";
+ save_str = "(null)";
len = strlen(save_str);
if (str + len + 1 < end)
memcpy(str, save_str, len + 1);
--
1.6.5.2.180.gc5b3e.dirty
Signed-off-by: André Goddard Rosa <[email protected]>
Acked-by: Frederic Weisbecker <[email protected]>
---
lib/vsprintf.c | 9 +++++----
1 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index fe56825..1e08b0d 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -1443,13 +1443,14 @@ do { \
case FORMAT_TYPE_STR: {
const char *save_str = va_arg(args, char *);
size_t len;
+
if ((unsigned long)save_str > (unsigned long)-PAGE_SIZE
|| (unsigned long)save_str < PAGE_SIZE)
save_str = "(null)";
- len = strlen(save_str);
- if (str + len + 1 < end)
- memcpy(str, save_str, len + 1);
- str += len + 1;
+ len = strlen(save_str) + 1;
+ if (str + len < end)
+ memcpy(str, save_str, len);
+ str += len;
break;
}
--
1.6.5.2.180.gc5b3e.dirty
Most relevant complaints were addressed.
Signed-off-by: André Goddard Rosa <[email protected]>
Acked-by: Frederic Weisbecker <[email protected]>
---
lib/vsprintf.c | 186 ++++++++++++++++++++++++++++++--------------------------
1 files changed, 99 insertions(+), 87 deletions(-)
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 1e08b0d..84fa8f4 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -9,7 +9,7 @@
* Wirzenius wrote this portably, Torvalds fucked it up :-)
*/
-/*
+/*
* Fri Jul 13 2001 Crutcher Dunnavant <[email protected]>
* - changed to provide snprintf and vsnprintf functions
* So Feb 1 16:51:32 CET 2004 Juergen Quade <[email protected]>
@@ -71,9 +71,9 @@ unsigned long simple_strtoul(const char *cp, char **endp, unsigned int base)
result = result * base + value;
cp++;
}
-
if (endp)
*endp = (char *)cp;
+
return result;
}
EXPORT_SYMBOL(simple_strtoul);
@@ -86,8 +86,9 @@ EXPORT_SYMBOL(simple_strtoul);
*/
long simple_strtol(const char *cp, char **endp, unsigned int base)
{
- if(*cp == '-')
+ if (*cp == '-')
return -simple_strtoul(cp + 1, endp, base);
+
return simple_strtoul(cp, endp, base);
}
EXPORT_SYMBOL(simple_strtol);
@@ -117,9 +118,9 @@ unsigned long long simple_strtoull(const char *cp, char **endp, unsigned int bas
result = result * base + value;
cp++;
}
-
if (endp)
*endp = (char *)cp;
+
return result;
}
EXPORT_SYMBOL(simple_strtoull);
@@ -132,8 +133,9 @@ EXPORT_SYMBOL(simple_strtoull);
*/
long long simple_strtoll(const char *cp, char **endp, unsigned int base)
{
- if(*cp=='-')
+ if (*cp == '-')
return -simple_strtoull(cp + 1, endp, base);
+
return simple_strtoull(cp, endp, base);
}
@@ -173,6 +175,7 @@ int strict_strtoul(const char *cp, unsigned int base, unsigned long *res)
val = simple_strtoul(cp, &tail, base);
if (tail == cp)
return -EINVAL;
+
if ((*tail == '\0') ||
((len == (size_t)(tail - cp) + 1) && (*tail == '\n'))) {
*res = val;
@@ -285,10 +288,11 @@ EXPORT_SYMBOL(strict_strtoll);
static int skip_atoi(const char **s)
{
- int i=0;
+ int i = 0;
while (isdigit(**s))
i = i*10 + *((*s)++) - '0';
+
return i;
}
@@ -302,7 +306,7 @@ static int skip_atoi(const char **s)
/* Formats correctly any integer in [0,99999].
* Outputs from one to five digits depending on input.
* On i386 gcc 4.1.2 -O2: ~250 bytes of code. */
-static char* put_dec_trunc(char *buf, unsigned q)
+static char *put_dec_trunc(char *buf, unsigned q)
{
unsigned d3, d2, d1, d0;
d1 = (q>>4) & 0xf;
@@ -331,14 +335,15 @@ static char* put_dec_trunc(char *buf, unsigned q)
d3 = d3 - 10*q;
*buf++ = d3 + '0'; /* next digit */
if (q != 0)
- *buf++ = q + '0'; /* most sign. digit */
+ *buf++ = q + '0'; /* most sign. digit */
}
}
}
+
return buf;
}
/* Same with if's removed. Always emits five digits */
-static char* put_dec_full(char *buf, unsigned q)
+static char *put_dec_full(char *buf, unsigned q)
{
/* BTW, if q is in [0,9999], 8-bit ints will be enough, */
/* but anyway, gcc produces better code with full-sized ints */
@@ -347,14 +352,15 @@ static char* put_dec_full(char *buf, unsigned q)
d2 = (q>>8) & 0xf;
d3 = (q>>12);
- /* Possible ways to approx. divide by 10 */
- /* gcc -O2 replaces multiply with shifts and adds */
- // (x * 0xcd) >> 11: 11001101 - shorter code than * 0x67 (on i386)
- // (x * 0x67) >> 10: 1100111
- // (x * 0x34) >> 9: 110100 - same
- // (x * 0x1a) >> 8: 11010 - same
- // (x * 0x0d) >> 7: 1101 - same, shortest code (on i386)
-
+ /*
+ * Possible ways to approx. divide by 10
+ * gcc -O2 replaces multiply with shifts and adds
+ * (x * 0xcd) >> 11: 11001101 - shorter code than * 0x67 (on i386)
+ * (x * 0x67) >> 10: 1100111
+ * (x * 0x34) >> 9: 110100 - same
+ * (x * 0x1a) >> 8: 11010 - same
+ * (x * 0x0d) >> 7: 1101 - same, shortest code (on i386)
+ */
d0 = 6*(d3 + d2 + d1) + (q & 0xf);
q = (d0 * 0xcd) >> 11;
d0 = d0 - 10*q;
@@ -375,10 +381,11 @@ static char* put_dec_full(char *buf, unsigned q)
d3 = d3 - 10*q;
*buf++ = d3 + '0';
*buf++ = q + '0';
+
return buf;
}
/* No inlining helps gcc to use registers better */
-static noinline char* put_dec(char *buf, unsigned long long num)
+static noinline char *put_dec(char *buf, unsigned long long num)
{
while (1) {
unsigned rem;
@@ -448,9 +455,9 @@ static char *number(char *buf, char *end, unsigned long long num,
spec.flags &= ~ZEROPAD;
sign = 0;
if (spec.flags & SIGN) {
- if ((signed long long) num < 0) {
+ if ((signed long long)num < 0) {
sign = '-';
- num = - (signed long long) num;
+ num = -(signed long long)num;
spec.field_width--;
} else if (spec.flags & PLUS) {
sign = '+';
@@ -478,7 +485,9 @@ static char *number(char *buf, char *end, unsigned long long num,
else if (spec.base != 10) { /* 8 or 16 */
int mask = spec.base - 1;
int shift = 3;
- if (spec.base == 16) shift = 4;
+
+ if (spec.base == 16)
+ shift = 4;
do {
tmp[i++] = (digits[((unsigned char)num) & mask] | locase);
num >>= shift;
@@ -493,7 +502,7 @@ static char *number(char *buf, char *end, unsigned long long num,
/* leading space padding */
spec.field_width -= spec.precision;
if (!(spec.flags & (ZEROPAD+LEFT))) {
- while(--spec.field_width >= 0) {
+ while (--spec.field_width >= 0) {
if (buf < end)
*buf = ' ';
++buf;
@@ -543,6 +552,7 @@ static char *number(char *buf, char *end, unsigned long long num,
*buf = ' ';
++buf;
}
+
return buf;
}
@@ -572,6 +582,7 @@ static char *string(char *buf, char *end, const char *s, struct printf_spec spec
*buf = ' ';
++buf;
}
+
return buf;
}
@@ -585,11 +596,13 @@ static char *symbol_string(char *buf, char *end, void *ptr,
sprint_symbol(sym, value);
else
kallsyms_lookup(value, NULL, NULL, NULL, sym);
+
return string(buf, end, sym, spec);
#else
- spec.field_width = 2*sizeof(void *);
+ spec.field_width = 2 * sizeof(void *);
spec.flags |= SPECIAL | SMALL | ZEROPAD;
spec.base = 16;
+
return number(buf, end, value, spec);
#endif
}
@@ -609,7 +622,7 @@ static char *resource_string(char *buf, char *end, struct resource *res,
.precision = -1,
.flags = SPECIAL | SMALL | ZEROPAD,
};
- /* room for the actual numbers, the two "0x", -, [, ] and the final zero */
+ /* room for actual numbers: the two "0x", -, [, ] and the final zero */
char sym[4*sizeof(resource_size_t) + 8];
char *p = sym, *pend = sym + sizeof(sym);
int size = -1;
@@ -666,22 +679,19 @@ static char *ip4_string(char *p, const u8 *addr, bool leading_zeros)
if (i < 3)
*p++ = '.';
}
-
*p = '\0';
+
return p;
}
static char *ip6_compressed_string(char *p, const char *addr)
{
- int i;
- int j;
- int range;
+ int i, j, range;
unsigned char zerolength[8];
int longest = 1;
int colonpos = -1;
u16 word;
- u8 hi;
- u8 lo;
+ u8 hi, lo;
bool needcolon = false;
bool useIPv4;
struct in6_addr in6;
@@ -748,22 +758,23 @@ static char *ip6_compressed_string(char *p, const char *addr)
*p++ = ':';
p = ip4_string(p, &in6.s6_addr[12], false);
}
-
*p = '\0';
+
return p;
}
static char *ip6_string(char *p, const char *addr, const char *fmt)
{
int i;
+
for (i = 0; i < 8; i++) {
p = pack_hex_byte(p, *addr++);
p = pack_hex_byte(p, *addr++);
if (fmt[0] == 'I' && i != 7)
*p++ = ':';
}
-
*p = '\0';
+
return p;
}
@@ -1269,7 +1280,8 @@ int vscnprintf(char *buf, size_t size, const char *fmt, va_list args)
{
int i;
- i=vsnprintf(buf,size,fmt,args);
+ i = vsnprintf(buf, size, fmt, args);
+
return (i >= size) ? (size - 1) : i;
}
EXPORT_SYMBOL(vscnprintf);
@@ -1288,14 +1300,15 @@ EXPORT_SYMBOL(vscnprintf);
*
* See the vsnprintf() documentation for format string extensions over C99.
*/
-int snprintf(char * buf, size_t size, const char *fmt, ...)
+int snprintf(char *buf, size_t size, const char *fmt, ...)
{
va_list args;
int i;
va_start(args, fmt);
- i=vsnprintf(buf,size,fmt,args);
+ i = vsnprintf(buf, size, fmt, args);
va_end(args);
+
return i;
}
EXPORT_SYMBOL(snprintf);
@@ -1311,7 +1324,7 @@ EXPORT_SYMBOL(snprintf);
* the trailing '\0'. If @size is <= 0 the function returns 0.
*/
-int scnprintf(char * buf, size_t size, const char *fmt, ...)
+int scnprintf(char *buf, size_t size, const char *fmt, ...)
{
va_list args;
int i;
@@ -1319,6 +1332,7 @@ int scnprintf(char * buf, size_t size, const char *fmt, ...)
va_start(args, fmt);
i = vsnprintf(buf, size, fmt, args);
va_end(args);
+
return (i >= size) ? (size - 1) : i;
}
EXPORT_SYMBOL(scnprintf);
@@ -1356,14 +1370,15 @@ EXPORT_SYMBOL(vsprintf);
*
* See the vsnprintf() documentation for format string extensions over C99.
*/
-int sprintf(char * buf, const char *fmt, ...)
+int sprintf(char *buf, const char *fmt, ...)
{
va_list args;
int i;
va_start(args, fmt);
- i=vsnprintf(buf, INT_MAX, fmt, args);
+ i = vsnprintf(buf, INT_MAX, fmt, args);
va_end(args);
+
return i;
}
EXPORT_SYMBOL(sprintf);
@@ -1421,7 +1436,6 @@ do { \
str += sizeof(type); \
} while (0)
-
while (*fmt) {
read = format_decode(fmt, &spec);
@@ -1509,8 +1523,8 @@ do { \
}
}
}
- return (u32 *)(PTR_ALIGN(str, sizeof(u32))) - bin_buf;
+ return (u32 *)(PTR_ALIGN(str, sizeof(u32))) - bin_buf;
#undef save_arg
}
EXPORT_SYMBOL_GPL(vbin_printf);
@@ -1542,7 +1556,6 @@ int bstr_printf(char *buf, size_t size, const char *fmt, const u32 *bin_buf)
unsigned long long num;
char *str, *end, c;
const char *args = (const char *)bin_buf;
-
struct printf_spec spec = {0};
if (WARN_ON_ONCE((int) size < 0))
@@ -1722,6 +1735,7 @@ int bprintf(u32 *bin_buf, size_t size, const char *fmt, ...)
va_start(args, fmt);
ret = vbin_printf(bin_buf, size, fmt, args);
va_end(args);
+
return ret;
}
EXPORT_SYMBOL_GPL(bprintf);
@@ -1734,18 +1748,16 @@ EXPORT_SYMBOL_GPL(bprintf);
* @fmt: format of buffer
* @args: arguments
*/
-int vsscanf(const char * buf, const char * fmt, va_list args)
+int vsscanf(const char *buf, const char *fmt, va_list args)
{
const char *str = buf;
char *next;
char digit;
int num = 0;
- int qualifier;
- int base;
- int field_width;
+ int qualifier, base, field_width;
int is_sign = 0;
- while(*fmt && *str) {
+ while (*fmt && *str) {
/* skip any white space in format */
/* white space in format matchs any amount of
* white space, including none, in the input.
@@ -1767,7 +1779,7 @@ int vsscanf(const char * buf, const char * fmt, va_list args)
if (!*fmt)
break;
++fmt;
-
+
/* skip this conversion.
* advance both strings to next white space
*/
@@ -1805,10 +1817,10 @@ int vsscanf(const char * buf, const char * fmt, va_list args)
if (!*fmt || !*str)
break;
- switch(*fmt++) {
+ switch (*fmt++) {
case 'c':
{
- char *s = (char *) va_arg(args,char*);
+ char *s = (char *)va_arg(args, char*);
if (field_width == -1)
field_width = 1;
do {
@@ -1819,17 +1831,16 @@ int vsscanf(const char * buf, const char * fmt, va_list args)
continue;
case 's':
{
- char *s = (char *) va_arg(args, char *);
- if(field_width == -1)
+ char *s = (char *)va_arg(args, char *);
+ if (field_width == -1)
field_width = INT_MAX;
/* first, skip leading white space in buffer */
while (isspace(*str))
str++;
/* now copy until next white space */
- while (*str && !isspace(*str) && field_width--) {
+ while (*str && !isspace(*str) && field_width--)
*s++ = *str++;
- }
*s = '\0';
num++;
}
@@ -1837,7 +1848,7 @@ int vsscanf(const char * buf, const char * fmt, va_list args)
case 'n':
/* return number of characters read so far */
{
- int *i = (int *)va_arg(args,int*);
+ int *i = (int *)va_arg(args, int*);
*i = str - buf;
}
continue;
@@ -1849,14 +1860,14 @@ int vsscanf(const char * buf, const char * fmt, va_list args)
base = 16;
break;
case 'i':
- base = 0;
+ base = 0;
case 'd':
is_sign = 1;
case 'u':
break;
case '%':
/* looking for '%' in str */
- if (*str++ != '%')
+ if (*str++ != '%')
return num;
continue;
default:
@@ -1875,63 +1886,63 @@ int vsscanf(const char * buf, const char * fmt, va_list args)
digit = *(str + 1);
if (!digit
- || (base == 16 && !isxdigit(digit))
- || (base == 10 && !isdigit(digit))
- || (base == 8 && (!isdigit(digit) || digit > '7'))
- || (base == 0 && !isdigit(digit)))
- break;
+ || (base == 16 && !isxdigit(digit))
+ || (base == 10 && !isdigit(digit))
+ || (base == 8 && (!isdigit(digit) || digit > '7'))
+ || (base == 0 && !isdigit(digit)))
+ break;
- switch(qualifier) {
+ switch (qualifier) {
case 'H': /* that's 'hh' in format */
if (is_sign) {
- signed char *s = (signed char *) va_arg(args,signed char *);
- *s = (signed char) simple_strtol(str,&next,base);
+ signed char *s = (signed char *)va_arg(args, signed char *);
+ *s = (signed char)simple_strtol(str, &next, base);
} else {
- unsigned char *s = (unsigned char *) va_arg(args, unsigned char *);
- *s = (unsigned char) simple_strtoul(str, &next, base);
+ unsigned char *s = (unsigned char *)va_arg(args, unsigned char *);
+ *s = (unsigned char)simple_strtoul(str, &next, base);
}
break;
case 'h':
if (is_sign) {
- short *s = (short *) va_arg(args,short *);
- *s = (short) simple_strtol(str,&next,base);
+ short *s = (short *)va_arg(args, short *);
+ *s = (short)simple_strtol(str, &next, base);
} else {
- unsigned short *s = (unsigned short *) va_arg(args, unsigned short *);
- *s = (unsigned short) simple_strtoul(str, &next, base);
+ unsigned short *s = (unsigned short *)va_arg(args, unsigned short *);
+ *s = (unsigned short)simple_strtoul(str, &next, base);
}
break;
case 'l':
if (is_sign) {
- long *l = (long *) va_arg(args,long *);
- *l = simple_strtol(str,&next,base);
+ long *l = (long *)va_arg(args, long *);
+ *l = simple_strtol(str, &next, base);
} else {
- unsigned long *l = (unsigned long*) va_arg(args,unsigned long*);
- *l = simple_strtoul(str,&next,base);
+ unsigned long *l = (unsigned long *)va_arg(args, unsigned long *);
+ *l = simple_strtoul(str, &next, base);
}
break;
case 'L':
if (is_sign) {
- long long *l = (long long*) va_arg(args,long long *);
- *l = simple_strtoll(str,&next,base);
+ long long *l = (long long *)va_arg(args, long long *);
+ *l = simple_strtoll(str, &next, base);
} else {
- unsigned long long *l = (unsigned long long*) va_arg(args,unsigned long long*);
- *l = simple_strtoull(str,&next,base);
+ unsigned long long *l = (unsigned long long *)va_arg(args, unsigned long long *);
+ *l = simple_strtoull(str, &next, base);
}
break;
case 'Z':
case 'z':
{
- size_t *s = (size_t*) va_arg(args,size_t*);
- *s = (size_t) simple_strtoul(str,&next,base);
+ size_t *s = (size_t *)va_arg(args, size_t *);
+ *s = (size_t)simple_strtoul(str, &next, base);
}
break;
default:
if (is_sign) {
- int *i = (int *) va_arg(args, int*);
- *i = (int) simple_strtol(str,&next,base);
+ int *i = (int *)va_arg(args, int *);
+ *i = (int)simple_strtol(str, &next, base);
} else {
- unsigned int *i = (unsigned int*) va_arg(args, unsigned int*);
- *i = (unsigned int) simple_strtoul(str,&next,base);
+ unsigned int *i = (unsigned int *)va_arg(args, unsigned int*);
+ *i = (unsigned int)simple_strtoul(str, &next, base);
}
break;
}
@@ -1962,14 +1973,15 @@ EXPORT_SYMBOL(vsscanf);
* @fmt: formatting of buffer
* @...: resulting arguments
*/
-int sscanf(const char * buf, const char * fmt, ...)
+int sscanf(const char *buf, const char *fmt, ...)
{
va_list args;
int i;
- va_start(args,fmt);
- i = vsscanf(buf,fmt,args);
+ va_start(args, fmt);
+ i = vsscanf(buf, fmt, args);
va_end(args);
+
return i;
}
EXPORT_SYMBOL(sscanf);
--
1.6.5.2.180.gc5b3e.dirty
It decreases code size as well:
text data bss dec hex filename
15758 0 8 15766 3d96 vsprintf.o (ex lib/lib.a-BEFORE)
15726 0 8 15734 3d76 vsprintf.o (ex lib/lib.a-TOLOWER)
Signed-off-by: André Goddard Rosa <[email protected]>
Acked-by: Frederic Weisbecker <[email protected]>
---
lib/vsprintf.c | 15 +++++++--------
1 files changed, 7 insertions(+), 8 deletions(-)
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 84fa8f4..d107583 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -981,8 +981,8 @@ precision:
qualifier:
/* get the conversion qualifier */
spec->qualifier = -1;
- if (*fmt == 'h' || *fmt == 'l' || *fmt == 'L' ||
- *fmt == 'Z' || *fmt == 'z' || *fmt == 't') {
+ if (*fmt == 'h' || TOLOWER(*fmt) == 'l' ||
+ TOLOWER(*fmt) == 'z' || *fmt == 't') {
spec->qualifier = *fmt++;
if (unlikely(spec->qualifier == *fmt)) {
if (spec->qualifier == 'l') {
@@ -1049,7 +1049,7 @@ qualifier:
spec->type = FORMAT_TYPE_LONG;
else
spec->type = FORMAT_TYPE_ULONG;
- } else if (spec->qualifier == 'Z' || spec->qualifier == 'z') {
+ } else if (TOLOWER(spec->qualifier) == 'z') {
spec->type = FORMAT_TYPE_SIZE_T;
} else if (spec->qualifier == 't') {
spec->type = FORMAT_TYPE_PTRDIFF;
@@ -1196,8 +1196,7 @@ int vsnprintf(char *buf, size_t size, const char *fmt, va_list args)
if (qualifier == 'l') {
long *ip = va_arg(args, long *);
*ip = (str - buf);
- } else if (qualifier == 'Z' ||
- qualifier == 'z') {
+ } else if (TOLOWER(qualifier) == 'z') {
size_t *ip = va_arg(args, size_t *);
*ip = (str - buf);
} else {
@@ -1487,7 +1486,7 @@ do { \
void *skip_arg;
if (qualifier == 'l')
skip_arg = va_arg(args, long *);
- else if (qualifier == 'Z' || qualifier == 'z')
+ else if (TOLOWER(qualifier) == 'z')
skip_arg = va_arg(args, size_t *);
else
skip_arg = va_arg(args, int *);
@@ -1798,8 +1797,8 @@ int vsscanf(const char *buf, const char *fmt, va_list args)
/* get conversion qualifier */
qualifier = -1;
- if (*fmt == 'h' || *fmt == 'l' || *fmt == 'L' ||
- *fmt == 'Z' || *fmt == 'z') {
+ if (*fmt == 'h' || TOLOWER(*fmt) == 'l' ||
+ TOLOWER(*fmt) == 'z') {
qualifier = *fmt++;
if (unlikely(qualifier == *fmt)) {
if (qualifier == 'h') {
--
1.6.5.2.180.gc5b3e.dirty
No functional change, just refactor the code so that it avoid checking
"if (hi)" two times in a sequence, taking advantage of previous check made.
It also reduces code size:
text data bss dec hex filename
15726 0 8 15734 3d76 vsprintf.o (ex lib/lib.a-BEFORE)
15710 0 8 15718 3d66 vsprintf.o (ex lib/lib.a-AFTER)
Signed-off-by: André Goddard Rosa <[email protected]>
Acked-by: Frederic Weisbecker <[email protected]>
---
lib/vsprintf.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index d107583..3c83f7b 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -745,8 +745,9 @@ static char *ip6_compressed_string(char *p, const char *addr)
p = pack_hex_byte(p, hi);
else
*p++ = hex_asc_lo(hi);
+ p = pack_hex_byte(p, lo);
}
- if (hi || lo > 0x0f)
+ else if (lo > 0x0f)
p = pack_hex_byte(p, lo);
else
*p++ = hex_asc_lo(lo);
--
1.6.5.2.180.gc5b3e.dirty
Cleanup by moving variables closer to the scope where they're used in fact.
Also, remove unneeded ones.
Signed-off-by: André Goddard Rosa <[email protected]>
Acked-by: Frederic Weisbecker <[email protected]>
---
lib/vsprintf.c | 64 ++++++++++++++++++++++++-------------------------------
1 files changed, 28 insertions(+), 36 deletions(-)
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 3c83f7b..8b60f77 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -840,8 +840,8 @@ static char *pointer(const char *fmt, char *buf, char *end, void *ptr,
case 'F':
case 'f':
ptr = dereference_function_descriptor(ptr);
- case 's':
/* Fallthrough */
+ case 's':
case 'S':
return symbol_string(buf, end, ptr, spec, *fmt);
case 'R':
@@ -1103,8 +1103,7 @@ qualifier:
int vsnprintf(char *buf, size_t size, const char *fmt, va_list args)
{
unsigned long long num;
- char *str, *end, c;
- int read;
+ char *str, *end;
struct printf_spec spec = {0};
/* Reject out-of-range values early. Large positive sizes are
@@ -1123,8 +1122,7 @@ int vsnprintf(char *buf, size_t size, const char *fmt, va_list args)
while (*fmt) {
const char *old_fmt = fmt;
-
- read = format_decode(fmt, &spec);
+ int read = format_decode(fmt, &spec);
fmt += read;
@@ -1148,7 +1146,9 @@ int vsnprintf(char *buf, size_t size, const char *fmt, va_list args)
spec.precision = va_arg(args, int);
break;
- case FORMAT_TYPE_CHAR:
+ case FORMAT_TYPE_CHAR: {
+ char c;
+
if (!(spec.flags & LEFT)) {
while (--spec.field_width > 0) {
if (str < end)
@@ -1167,6 +1167,7 @@ int vsnprintf(char *buf, size_t size, const char *fmt, va_list args)
++str;
}
break;
+ }
case FORMAT_TYPE_STR:
str = string(str, end, va_arg(args, char *), spec);
@@ -1411,7 +1412,6 @@ int vbin_printf(u32 *bin_buf, size_t size, const char *fmt, va_list args)
{
struct printf_spec spec = {0};
char *str, *end;
- int read;
str = (char *)bin_buf;
end = (char *)(bin_buf + size);
@@ -1437,12 +1437,14 @@ do { \
} while (0)
while (*fmt) {
- read = format_decode(fmt, &spec);
+ int read = format_decode(fmt, &spec);
fmt += read;
switch (spec.type) {
case FORMAT_TYPE_NONE:
+ case FORMAT_TYPE_INVALID:
+ case FORMAT_TYPE_PERCENT_CHAR:
break;
case FORMAT_TYPE_WIDTH:
@@ -1475,12 +1477,6 @@ do { \
fmt++;
break;
- case FORMAT_TYPE_PERCENT_CHAR:
- break;
-
- case FORMAT_TYPE_INVALID:
- break;
-
case FORMAT_TYPE_NRCHARS: {
/* skip %n 's argument */
int qualifier = spec.qualifier;
@@ -1553,10 +1549,9 @@ EXPORT_SYMBOL_GPL(vbin_printf);
*/
int bstr_printf(char *buf, size_t size, const char *fmt, const u32 *bin_buf)
{
- unsigned long long num;
- char *str, *end, c;
- const char *args = (const char *)bin_buf;
struct printf_spec spec = {0};
+ char *str, *end;
+ const char *args = (const char *)bin_buf;
if (WARN_ON_ONCE((int) size < 0))
return 0;
@@ -1586,10 +1581,8 @@ int bstr_printf(char *buf, size_t size, const char *fmt, const u32 *bin_buf)
}
while (*fmt) {
- int read;
const char *old_fmt = fmt;
-
- read = format_decode(fmt, &spec);
+ int read = format_decode(fmt, &spec);
fmt += read;
@@ -1613,7 +1606,9 @@ int bstr_printf(char *buf, size_t size, const char *fmt, const u32 *bin_buf)
spec.precision = get_arg(int);
break;
- case FORMAT_TYPE_CHAR:
+ case FORMAT_TYPE_CHAR: {
+ char c;
+
if (!(spec.flags & LEFT)) {
while (--spec.field_width > 0) {
if (str < end)
@@ -1631,11 +1626,11 @@ int bstr_printf(char *buf, size_t size, const char *fmt, const u32 *bin_buf)
++str;
}
break;
+ }
case FORMAT_TYPE_STR: {
const char *str_arg = args;
- size_t len = strlen(str_arg);
- args += len + 1;
+ args += strlen(str_arg) + 1;
str = string(str, end, (char *)str_arg, spec);
break;
}
@@ -1647,11 +1642,6 @@ int bstr_printf(char *buf, size_t size, const char *fmt, const u32 *bin_buf)
break;
case FORMAT_TYPE_PERCENT_CHAR:
- if (str < end)
- *str = '%';
- ++str;
- break;
-
case FORMAT_TYPE_INVALID:
if (str < end)
*str = '%';
@@ -1662,15 +1652,15 @@ int bstr_printf(char *buf, size_t size, const char *fmt, const u32 *bin_buf)
/* skip */
break;
- default:
+ default: {
+ unsigned long long num;
+
switch (spec.type) {
case FORMAT_TYPE_LONG_LONG:
num = get_arg(long long);
break;
case FORMAT_TYPE_ULONG:
- num = get_arg(unsigned long);
- break;
case FORMAT_TYPE_LONG:
num = get_arg(unsigned long);
break;
@@ -1700,8 +1690,9 @@ int bstr_printf(char *buf, size_t size, const char *fmt, const u32 *bin_buf)
}
str = number(str, end, num, spec);
- }
- }
+ } /* default: */
+ } /* switch(spec.type) */
+ } /* while(*fmt) */
if (size > 0) {
if (str < end)
@@ -1755,7 +1746,7 @@ int vsscanf(const char *buf, const char *fmt, va_list args)
char digit;
int num = 0;
int qualifier, base, field_width;
- int is_sign = 0;
+ bool is_sign;
while (*fmt && *str) {
/* skip any white space in format */
@@ -1811,12 +1802,13 @@ int vsscanf(const char *buf, const char *fmt, va_list args)
}
}
}
- base = 10;
- is_sign = 0;
if (!*fmt || !*str)
break;
+ base = 10;
+ is_sign = 0;
+
switch (*fmt++) {
case 'c':
{
--
1.6.5.2.180.gc5b3e.dirty
When converting more caller sites, the inline decision will be left up to gcc.
It decreases code size:
text data bss dec hex filename
15710 0 8 15718 3d66 vsprintf.o (ex lib/lib.a-BEFORE)
15534 0 8 15542 3cb6 vsprintf.o (ex lib/lib.a-AFTER)
Signed-off-by: André Goddard Rosa <[email protected]>
Acked-by: Frederic Weisbecker <[email protected]>
---
lib/vsprintf.c | 19 +++++++++++--------
1 files changed, 11 insertions(+), 8 deletions(-)
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 8b60f77..f6492b5 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -1733,6 +1733,13 @@ EXPORT_SYMBOL_GPL(bprintf);
#endif /* CONFIG_BINARY_PRINTF */
+static noinline char *skip_space(const char *str)
+{
+ while (isspace(*str))
+ ++str;
+ return (char *)str;
+}
+
/**
* vsscanf - Unformat a buffer into a list of arguments
* @buf: input buffer
@@ -1754,10 +1761,8 @@ int vsscanf(const char *buf, const char *fmt, va_list args)
* white space, including none, in the input.
*/
if (isspace(*fmt)) {
- while (isspace(*fmt))
- ++fmt;
- while (isspace(*str))
- ++str;
+ fmt = skip_space(fmt);
+ str = skip_space(str);
}
/* anything that is not a conversion must match exactly */
@@ -1827,8 +1832,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 */
- while (isspace(*str))
- str++;
+ str = skip_space(str);
/* now copy until next white space */
while (*str && !isspace(*str) && field_width--)
@@ -1870,8 +1874,7 @@ int vsscanf(const char *buf, const char *fmt, va_list args)
/* have some sort of integer conversion.
* first, skip white space in buffer.
*/
- while (isspace(*str))
- str++;
+ str = skip_space(str);
digit = *str;
if (is_sign && digit == '-')
--
1.6.5.2.180.gc5b3e.dirty
The difference between simple_strtoul() and simple_strtoull() is just
the size of the variable used to keep track of the sum of characters
converted to numbers:
unsigned long simple_strtoul() {...}
unsigned long long simple_strtoull(){...}
Both are same size on my Core 2/gcc 4.4.1.
Overflow condition is not checked on both functions, so an extremely large
string can break these functions so that they don't even notice it.
As we do not care for overflowing on these functions, always keep the sum
using the larger variable around (unsigned long long) on simple_strtoull()
and cast it to (unsigned long) on simple_strtoul(), which then becomes
just a wrapper around simple_strtoull().
Code size decreases by 304 bytes:
text data bss dec hex filename
15534 0 8 15542 3cb6 vsprintf.o (ex lib/lib.a-BEFORE)
15230 0 8 15238 3b86 vsprintf.o (ex lib/lib.a-AFTER)
Signed-off-by: André Goddard Rosa <[email protected]>
---
lib/vsprintf.c | 48 ++++++++++++++----------------------------------
1 files changed, 14 insertions(+), 34 deletions(-)
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index f6492b5..7799a6e 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -47,14 +47,14 @@ static unsigned int simple_guess_base(const char *cp)
}
/**
- * simple_strtoul - convert a string to an unsigned long
+ * simple_strtoull - convert a string to an unsigned long long
* @cp: The start of the string
* @endp: A pointer to the end of the parsed string will be placed here
* @base: The number base to use
*/
-unsigned long simple_strtoul(const char *cp, char **endp, unsigned int base)
+unsigned long long simple_strtoull(const char *cp, char **endp, unsigned int base)
{
- unsigned long result = 0;
+ unsigned long long result = 0;
if (!base)
base = simple_guess_base(cp);
@@ -76,54 +76,34 @@ unsigned long simple_strtoul(const char *cp, char **endp, unsigned int base)
return result;
}
-EXPORT_SYMBOL(simple_strtoul);
+EXPORT_SYMBOL(simple_strtoull);
/**
- * simple_strtol - convert a string to a signed long
+ * simple_strtoul - convert a string to an unsigned long
* @cp: The start of the string
* @endp: A pointer to the end of the parsed string will be placed here
* @base: The number base to use
*/
-long simple_strtol(const char *cp, char **endp, unsigned int base)
+unsigned long simple_strtoul(const char *cp, char **endp, unsigned int base)
{
- if (*cp == '-')
- return -simple_strtoul(cp + 1, endp, base);
-
- return simple_strtoul(cp, endp, base);
+ return simple_strtoull(cp, endp, base);
}
-EXPORT_SYMBOL(simple_strtol);
+EXPORT_SYMBOL(simple_strtoul);
/**
- * simple_strtoull - convert a string to an unsigned long long
+ * simple_strtol - convert a string to a signed long
* @cp: The start of the string
* @endp: A pointer to the end of the parsed string will be placed here
* @base: The number base to use
*/
-unsigned long long simple_strtoull(const char *cp, char **endp, unsigned int base)
+long simple_strtol(const char *cp, char **endp, unsigned int base)
{
- unsigned long long result = 0;
-
- if (!base)
- base = simple_guess_base(cp);
-
- if (base == 16 && cp[0] == '0' && TOLOWER(cp[1]) == 'x')
- cp += 2;
-
- while (isxdigit(*cp)) {
- unsigned int value;
-
- value = isdigit(*cp) ? *cp - '0' : TOLOWER(*cp) - 'a' + 10;
- if (value >= base)
- break;
- result = result * base + value;
- cp++;
- }
- if (endp)
- *endp = (char *)cp;
+ if (*cp == '-')
+ return -simple_strtoul(cp + 1, endp, base);
- return result;
+ return simple_strtoul(cp, endp, base);
}
-EXPORT_SYMBOL(simple_strtoull);
+EXPORT_SYMBOL(simple_strtol);
/**
* simple_strtoll - convert a string to a signed long long
--
1.6.5.2.180.gc5b3e.dirty
While at it, use tabs to indent the comments.
Signed-off-by: André Goddard Rosa <[email protected]>
---
include/linux/ctype.h | 2 +-
lib/ctype.c | 50 ++++++++++++++++++++++++------------------------
2 files changed, 26 insertions(+), 26 deletions(-)
diff --git a/include/linux/ctype.h b/include/linux/ctype.h
index afa3639..6dec944 100644
--- a/include/linux/ctype.h
+++ b/include/linux/ctype.h
@@ -15,7 +15,7 @@
#define _X 0x40 /* hex digit */
#define _SP 0x80 /* hard space (0x20) */
-extern unsigned char _ctype[];
+extern const unsigned char _ctype[];
#define __ismask(x) (_ctype[(int)(unsigned char)(x)])
diff --git a/lib/ctype.c b/lib/ctype.c
index d02ace1..26baa62 100644
--- a/lib/ctype.c
+++ b/lib/ctype.c
@@ -7,30 +7,30 @@
#include <linux/ctype.h>
#include <linux/module.h>
-unsigned char _ctype[] = {
-_C,_C,_C,_C,_C,_C,_C,_C, /* 0-7 */
-_C,_C|_S,_C|_S,_C|_S,_C|_S,_C|_S,_C,_C, /* 8-15 */
-_C,_C,_C,_C,_C,_C,_C,_C, /* 16-23 */
-_C,_C,_C,_C,_C,_C,_C,_C, /* 24-31 */
-_S|_SP,_P,_P,_P,_P,_P,_P,_P, /* 32-39 */
-_P,_P,_P,_P,_P,_P,_P,_P, /* 40-47 */
-_D,_D,_D,_D,_D,_D,_D,_D, /* 48-55 */
-_D,_D,_P,_P,_P,_P,_P,_P, /* 56-63 */
-_P,_U|_X,_U|_X,_U|_X,_U|_X,_U|_X,_U|_X,_U, /* 64-71 */
-_U,_U,_U,_U,_U,_U,_U,_U, /* 72-79 */
-_U,_U,_U,_U,_U,_U,_U,_U, /* 80-87 */
-_U,_U,_U,_P,_P,_P,_P,_P, /* 88-95 */
-_P,_L|_X,_L|_X,_L|_X,_L|_X,_L|_X,_L|_X,_L, /* 96-103 */
-_L,_L,_L,_L,_L,_L,_L,_L, /* 104-111 */
-_L,_L,_L,_L,_L,_L,_L,_L, /* 112-119 */
-_L,_L,_L,_P,_P,_P,_P,_C, /* 120-127 */
-0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0, /* 128-143 */
-0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0, /* 144-159 */
-_S|_SP,_P,_P,_P,_P,_P,_P,_P,_P,_P,_P,_P,_P,_P,_P,_P, /* 160-175 */
-_P,_P,_P,_P,_P,_P,_P,_P,_P,_P,_P,_P,_P,_P,_P,_P, /* 176-191 */
-_U,_U,_U,_U,_U,_U,_U,_U,_U,_U,_U,_U,_U,_U,_U,_U, /* 192-207 */
-_U,_U,_U,_U,_U,_U,_U,_P,_U,_U,_U,_U,_U,_U,_U,_L, /* 208-223 */
-_L,_L,_L,_L,_L,_L,_L,_L,_L,_L,_L,_L,_L,_L,_L,_L, /* 224-239 */
-_L,_L,_L,_L,_L,_L,_L,_P,_L,_L,_L,_L,_L,_L,_L,_L}; /* 240-255 */
+const unsigned char _ctype[] = {
+_C,_C,_C,_C,_C,_C,_C,_C, /* 0-7 */
+_C,_C|_S,_C|_S,_C|_S,_C|_S,_C|_S,_C,_C, /* 8-15 */
+_C,_C,_C,_C,_C,_C,_C,_C, /* 16-23 */
+_C,_C,_C,_C,_C,_C,_C,_C, /* 24-31 */
+_S|_SP,_P,_P,_P,_P,_P,_P,_P, /* 32-39 */
+_P,_P,_P,_P,_P,_P,_P,_P, /* 40-47 */
+_D,_D,_D,_D,_D,_D,_D,_D, /* 48-55 */
+_D,_D,_P,_P,_P,_P,_P,_P, /* 56-63 */
+_P,_U|_X,_U|_X,_U|_X,_U|_X,_U|_X,_U|_X,_U, /* 64-71 */
+_U,_U,_U,_U,_U,_U,_U,_U, /* 72-79 */
+_U,_U,_U,_U,_U,_U,_U,_U, /* 80-87 */
+_U,_U,_U,_P,_P,_P,_P,_P, /* 88-95 */
+_P,_L|_X,_L|_X,_L|_X,_L|_X,_L|_X,_L|_X,_L, /* 96-103 */
+_L,_L,_L,_L,_L,_L,_L,_L, /* 104-111 */
+_L,_L,_L,_L,_L,_L,_L,_L, /* 112-119 */
+_L,_L,_L,_P,_P,_P,_P,_C, /* 120-127 */
+0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0, /* 128-143 */
+0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0, /* 144-159 */
+_S|_SP,_P,_P,_P,_P,_P,_P,_P,_P,_P,_P,_P,_P,_P,_P,_P, /* 160-175 */
+_P,_P,_P,_P,_P,_P,_P,_P,_P,_P,_P,_P,_P,_P,_P,_P, /* 176-191 */
+_U,_U,_U,_U,_U,_U,_U,_U,_U,_U,_U,_U,_U,_U,_U,_U, /* 192-207 */
+_U,_U,_U,_U,_U,_U,_U,_P,_U,_U,_U,_U,_U,_U,_U,_L, /* 208-223 */
+_L,_L,_L,_L,_L,_L,_L,_L,_L,_L,_L,_L,_L,_L,_L,_L, /* 224-239 */
+_L,_L,_L,_L,_L,_L,_L,_P,_L,_L,_L,_L,_L,_L,_L,_L}; /* 240-255 */
EXPORT_SYMBOL(_ctype);
--
1.6.5.2.180.gc5b3e.dirty
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 most common case (non-null string).
Fixed const return as noticed by Jan Engelhardt and James Bottomley.
Fixed unnecessary extra cast on strstrip() as noticed by Jan Engelhardt.
Signed-off-by: André Goddard Rosa <[email protected]>
---
include/linux/ctype.h | 1 +
include/linux/string.h | 1 +
lib/string.c | 19 +++++++++++++++----
3 files changed, 17 insertions(+), 4 deletions(-)
diff --git a/include/linux/ctype.h b/include/linux/ctype.h
index 6dec944..a3d6ee0 100644
--- a/include/linux/ctype.h
+++ b/include/linux/ctype.h
@@ -27,6 +27,7 @@ extern const unsigned char _ctype[];
#define islower(c) ((__ismask(c)&(_L)) != 0)
#define isprint(c) ((__ismask(c)&(_P|_U|_L|_D|_SP)) != 0)
#define ispunct(c) ((__ismask(c)&(_P)) != 0)
+/* Note: isspace() must return false for %NUL-terminator */
#define isspace(c) ((__ismask(c)&(_S)) != 0)
#define isupper(c) ((__ismask(c)&(_U)) != 0)
#define isxdigit(c) ((__ismask(c)&(_D|_X)) != 0)
diff --git a/include/linux/string.h b/include/linux/string.h
index b850886..168dad1 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 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..087d33b 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.
+ */
+char *skip_spaces(const char *str)
+{
+ while (isspace(*str))
+ ++str;
+ return (char *)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 skip_spaces(s);
}
EXPORT_SYMBOL(strstrip);
--
1.6.5.2.180.gc5b3e.dirty
... so that strlen() iterates over a smaller string comprising of the
remaining characters only.
Signed-off-by: André Goddard Rosa <[email protected]>
---
lib/string.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/lib/string.c b/lib/string.c
index 087d33b..c31d0f4 100644
--- a/lib/string.c
+++ b/lib/string.c
@@ -356,8 +356,8 @@ char *strstrip(char *s)
size_t size;
char *end;
+ s = skip_spaces(s);
size = strlen(s);
-
if (!size)
return s;
@@ -366,7 +366,7 @@ char *strstrip(char *s)
end--;
*(end + 1) = '\0';
- return skip_spaces(s);
+ return s;
}
EXPORT_SYMBOL(strstrip);
--
1.6.5.2.180.gc5b3e.dirty
Makes use of skip_spaces() defined in lib/string.c for removing leading
spaces from strings all over the tree.
It decreases lib.a code size by 47 bytes and reuses the function tree-wide:
text data bss dec hex filename
64688 584 592 65864 10148 (TOTALS-BEFORE)
64641 584 592 65817 10119 (TOTALS-AFTER)
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".
Julia Lawall tried the semantic patch (http://coccinelle.lip6.fr) below,
and found occurrences of this pattern on 3 more files:
drivers/leds/led-class.c
drivers/leds/ledtrig-timer.c
drivers/video/output.c
@@
expression str;
@@
( // ignore skip_spaces cases
while (*str && isspace(*str)) { \(str++;\|++str;\) }
|
- *str &&
isspace(*str)
)
Signed-off-by: André Goddard Rosa <[email protected]>
cc: Julia Lawall <[email protected]>
---
arch/s390/kernel/debug.c | 3 +-
arch/um/drivers/mconsole_kern.c | 16 ++++++--------
arch/x86/kernel/cpu/mtrr/if.c | 11 +++------
drivers/leds/led-class.c | 2 +-
drivers/leds/ledtrig-timer.c | 4 +-
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 +-
drivers/video/output.c | 2 +-
fs/cachefiles/daemon.c | 4 +-
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 ++---
24 files changed, 66 insertions(+), 115 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/leds/led-class.c b/drivers/leds/led-class.c
index f2cc13d..782f958 100644
--- a/drivers/leds/led-class.c
+++ b/drivers/leds/led-class.c
@@ -50,7 +50,7 @@ static ssize_t led_brightness_store(struct device *dev,
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 --git a/drivers/leds/ledtrig-timer.c b/drivers/leds/ledtrig-timer.c
index 3b83406..38b3378 100644
--- a/drivers/leds/ledtrig-timer.c
+++ b/drivers/leds/ledtrig-timer.c
@@ -83,7 +83,7 @@ static ssize_t led_delay_on_store(struct device *dev,
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(struct device *dev,
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 --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 e64c971..01468a1 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 d93108d..e8026d9 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/drivers/video/output.c b/drivers/video/output.c
index 5e6439a..5137aa0 100644
--- a/drivers/video/output.c
+++ b/drivers/video/output.c
@@ -50,7 +50,7 @@ static ssize_t video_output_store_state(struct device *dev,
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;
diff --git a/fs/cachefiles/daemon.c b/fs/cachefiles/daemon.c
index 4618516..c241356 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"
@@ -257,8 +258,7 @@ static ssize_t cachefiles_daemon_write(struct file *file,
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 d4ca92a..8b9e3bb 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -2099,11 +2099,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 7799a6e..1eb66dd 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -1713,13 +1713,6 @@ EXPORT_SYMBOL_GPL(bprintf);
#endif /* CONFIG_BINARY_PRINTF */
-static noinline char *skip_space(const char *str)
-{
- while (isspace(*str))
- ++str;
- return (char *)str;
-}
-
/**
* vsscanf - Unformat a buffer into a list of arguments
* @buf: input buffer
@@ -1741,8 +1734,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 */
@@ -1812,7 +1805,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--)
@@ -1854,7 +1847,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.180.gc5b3e.dirty
On 2009-11-15 08:15:03, Andr? Goddard Rosa wrote:
> +char *skip_spaces(const char *str)
> +{
> + while (isspace(*str))
> + ++str;
> + return (char *)str;
> +}
Is there a good reason why the parameter 'str' is declared 'const'
when skip_spaces() returns a non-const pointer into str ?
Cheers
Anders
On 2009-11-15 21:02:15, Anonymous wrote:
> On Sun, Nov 15, 2009 at 08:33:29PM +0100, Anders Larsen wrote:
> > On 2009-11-15 08:15:03, Andr? Goddard Rosa wrote:
> > >+char *skip_spaces(const char *str)
> > >+{
> > >+ while (isspace(*str))
> > >+ ++str;
> > >+ return (char *)str;
> > >+}
> >> Is there a good reason why the parameter 'str' is declared 'const'
> > when skip_spaces() returns a non-const pointer into str ?
> >Declaring return value as const won't let us modificate string after
> skipping spaces. Declaring parameter as non-const won't let us
> giving (const char *) to this function. So i think it is ok.
skip_spaces() _implicitly_ casts away the 'const' of the parameter,
which may come as a (nasty) surprise to users of the function.
Consider this (contrieved and buggy) example:
const char* my_string = " do not modify me! ";
char* result = strcat(skip_spaces(my_string, "boom!"));
The proposed implementation of skip_spaces() effectively prevents
the compiler from catching this obvious bug.
Cheers
Anders
Hi, Anders!
On Sun, Nov 15, 2009 at 7:51 PM, Anders Larsen <[email protected]> wrote:
> On 2009-11-15 21:02:15, Anonymous wrote:
>>
>> On Sun, Nov 15, 2009 at 08:33:29PM +0100, Anders Larsen wrote:
>> > On 2009-11-15 08:15:03, Andr? Goddard Rosa wrote:
>> > >+char *skip_spaces(const char *str)
>> > >+{
>> > >+ ? ?while (isspace(*str))
>> > >+ ? ? ? ? ? ?++str;
>> > >+ ? ?return (char *)str;
>> > >+}
>> >> Is there a good reason why the parameter 'str' is declared 'const'
>> > when skip_spaces() returns a non-const pointer into str ?
>> >Declaring return value as const won't let us modificate string after
>> skipping spaces. Declaring parameter as non-const won't let us
>> giving (const char *) to this function. So i think it is ok.
>
> skip_spaces() _implicitly_ casts away the 'const' of the parameter,
> which may come as a (nasty) surprise to users of the function.
>
> Consider this (contrieved and buggy) example:
>
> ? ? ? ?const char* my_string = " do not modify me! ";
> ? ? ? ?char* result = strcat(skip_spaces(my_string, "boom!"));
>
> The proposed implementation of skip_spaces() effectively prevents
> the compiler from catching this obvious bug.
>
I agree with the above comment. It's possible to make it differently
char* skip_spaces(char *),
but it's necessary to change many of its callers at the same time:
CC lib/argv_split.o
lib/argv_split.c: In function ?count_argc?:
lib/argv_split.c:24: warning: passing argument 1 of ?skip_spaces?
discards qualifiers from pointer target type
include/linux/string.h:65: note: expected ?char *? but argument is of
type ?const char *?
lib/argv_split.c: In function ?argv_split?:
lib/argv_split.c:78: warning: passing argument 1 of ?skip_spaces?
discards qualifiers from pointer target type
include/linux/string.h:65: note: expected ?char *? but argument is of
type ?const char *?
...
CC lib/vsprintf.o
lib/vsprintf.c: In function ?vsscanf?:
lib/vsprintf.c:1737: warning: passing argument 1 of ?skip_spaces?
discards qualifiers from pointer target type
include/linux/string.h:65: note: expected ?char *? but argument is of
type ?const char *?
lib/vsprintf.c:1738: warning: passing argument 1 of ?skip_spaces?
discards qualifiers from pointer target type
include/linux/string.h:65: note: expected ?char *? but argument is of
type ?const char *?
lib/vsprintf.c:1808: warning: passing argument 1 of ?skip_spaces?
discards qualifiers from pointer target type
include/linux/string.h:65: note: expected ?char *? but argument is of
type ?const char *?
lib/vsprintf.c:1850: warning: passing argument 1 of ?skip_spaces?
discards qualifiers from pointer target type
include/linux/string.h:65: note: expected ?char *? but argument is of
type ?const char *?
There are other functions following the same pattern presently:
char *strstr(const char *haystack, const char *needle);
char *strchr(const char *s, int c);
char *strrchr(const char *s, int c);
on both glibc and the kernel.
Thanks,
Andr?
> diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
> index d93108d..e8026d9 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;
>
Acked-by: Henrique de Moraes Holschuh <[email protected]>
(thinkpad-acpi changes only)
--
"One disk to rule them all, One disk to find them. One disk to bring
them all and in the darkness grind them. In the Land of Redmond
where the shadows lie." -- The Silicon Valley Tarot
Henrique Holschuh