[RFC/PATCH 0/8] Introduce strreplace

[RFC/PATCH 0/8] Introduce strreplace

[Rasmus Villemoes]

Doing single-character substitution on an entire string is open-coded
in a few places, sometimes in a rather suboptimal way. This introduces
a trivial helper, strreplace, for this task along with a few example
conversions.

Rasmus Villemoes (8):
  lib: string: Introduce strreplace
  kernel/trace/trace_events_filter.c: Use strreplace
  blktrace: use strreplace in do_blk_trace_setup
  lib/kobject.c: Use strreplace
  drivers/base/core.c: Use strreplace
  drivers/md/md.c: Use strreplace
  fs/jbd2/journal.c: Use strreplace
  fs/ext4/super.c: Use strreplace in ext4_fill_super

 drivers/base/core.c                |  5 +----
 drivers/md/md.c                    |  4 +---
 fs/ext4/super.c                    |  4 +---
 fs/jbd2/journal.c                  | 10 ++--------
 include/linux/string.h             |  1 +
 kernel/trace/blktrace.c            |  6 ++----
 kernel/trace/trace_events_filter.c |  5 ++---
 lib/kobject.c                      |  4 +---
 lib/string.c                       | 17 +++++++++++++++++
 9 files changed, 28 insertions(+), 28 deletions(-)

-- 
2.1.3

[RFC/PATCH 1/8] lib: string: Introduce strreplace

[Rasmus Villemoes]

Strings are sometimes sanitized by replacing a certain character
(often '/') by another (often '!'). In a few places, this is done the
same way Schlemiel the Painter would do it. Others are slightly
smarter but still do multiple strchr() calls. Introduce strreplace()
to do this using a single function call and a single pass over the
string.

One would expect the return value to be one of three things: void, s,
or the number of replacements made. I chose the fourth, returning a
pointer to the end of the string. This is more likely to be useful
(for example allowing the caller to avoid a strlen call).

Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
 include/linux/string.h |  1 +
 lib/string.c           | 17 +++++++++++++++++
 2 files changed, 18 insertions(+)

diff --git a/include/linux/string.h b/include/linux/string.h
index e40099e585c9..4f09e41c7169 100644
--- a/include/linux/string.h
+++ b/include/linux/string.h
@@ -111,6 +111,7 @@ extern int memcmp(const void *,const void *,__kernel_size_t);
 extern void * memchr(const void *,int,__kernel_size_t);
 #endif
 void *memchr_inv(const void *s, int c, size_t n);
+extern char *strreplace(char *s, char bad, char good);
 
 extern void kfree_const(const void *x);
 
diff --git a/lib/string.c b/lib/string.c
index bb3d4b6993c4..ef52e86f6b87 100644
--- a/lib/string.c
+++ b/lib/string.c
@@ -849,3 +849,20 @@ void *memchr_inv(const void *start, int c, size_t bytes)
 	return check_bytes8(start, value, bytes % 8);
 }
 EXPORT_SYMBOL(memchr_inv);
+
+/**
+ * strreplace - Replace all occurences of character in string
+ * @s: The string to operate on
+ * @bad: The character being replaced
+ * @good: The character @bad is replaced with.
+ *
+ * Returns pointer to the nul byte at the end of @s.
+ */
+char *strreplace(char *s, char bad, char good)
+{
+	for (; *s; ++s)
+		if (*s == bad)
+			*s = good;
+	return s;
+}
+EXPORT_SYMBOL(strreplace);
-- 
2.1.3

Re: [RFC/PATCH 1/8] lib: string: Introduce strreplace

[Joe Perches]

On Thu, 2015-06-04 at 11:37 +0200, Rasmus Villemoes wrote:
> Strings are sometimes sanitized by replacing a certain character
> (often '/') by another (often '!'). In a few places, this is done the
> same way Schlemiel the Painter would do it.

:)

> Others are slightly
> smarter but still do multiple strchr() calls. Introduce strreplace()
> to do this using a single function call and a single pass over the
> string.
> 
> One would expect the return value to be one of three things: void, s,
> or the number of replacements made. I chose the fourth, returning a
> pointer to the end of the string. This is more likely to be useful
> (for example allowing the caller to avoid a strlen call).

You used in one of the follow-on patches too.

> diff --git a/lib/string.c b/lib/string.c
[]
> @@ -849,3 +849,20 @@ void *memchr_inv(const void *start, int c, size_t bytes)
>  	return check_bytes8(start, value, bytes % 8);
>  }
>  EXPORT_SYMBOL(memchr_inv);
> +
> +/**
> + * strreplace - Replace all occurences of character in string

occurrences

> + * @s: The string to operate on
> + * @bad: The character being replaced
> + * @good: The character @bad is replaced with.
> + *
> + * Returns pointer to the nul byte at the end of @s.
> + */
> +char *strreplace(char *s, char bad, char good)
> +{
> +	for (; *s; ++s)
> +		if (*s == bad)
> +			*s = good;
> +	return s;
> +}
> +EXPORT_SYMBOL(strreplace);

Seems sensible, but the name maybe could be be more
explicit as strreplace seems like it should more like
a string substitution rather than a char substitution.

Maybe strsubstchr or something like it (strtranschr?)

Because it's so tiny, perhaps this could be inline
instead of EXPORT_SYMBOL.

Maybe from and to instead of good and bad arguments.

Re: [RFC/PATCH 1/8] lib: string: Introduce strreplace

[Al Viro]

On Thu, Jun 04, 2015 at 03:56:07AM -0700, Joe Perches wrote:
> Seems sensible, but the name maybe could be be more
> explicit as strreplace seems like it should more like
> a string substitution rather than a char substitution.
> 
> Maybe strsubstchr or something like it (strtranschr?)

Why not go the full monty and call it strtrtsstrrrtst(), with
strtrtssstrrtst() doing almost, but not quite the same thing?

Re: [RFC/PATCH 1/8] lib: string: Introduce strreplace

[Joe Perches]

On Thu, 2015-06-04 at 23:37 +0100, Al Viro wrote:
> On Thu, Jun 04, 2015 at 03:56:07AM -0700, Joe Perches wrote:
> > Seems sensible, but the name maybe could be be more
> > explicit as strreplace seems like it should more like
> > a string substitution rather than a char substitution.
> > 
> > Maybe strsubstchr or something like it (strtranschr?)
> 
> Why not go the full monty and call it strtrtsstrrrtst(), with
> strtrtssstrrtst() doing almost, but not quite the same thing?

Presumably the extra r is for reverse and the
extra s in for insensitive, just because...

Naming does matter.  Consistency too.

And for the consistency bit, the char arguments
should probably be int.

[RFC/PATCH 2/8] kernel/trace/trace_events_filter.c: Use strreplace

[Rasmus Villemoes]

There's no point in starting over every time we see a ','...

Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
 kernel/trace/trace_events_filter.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/kernel/trace/trace_events_filter.c b/kernel/trace/trace_events_filter.c
index ced69da0ff55..a987601d7b43 100644
--- a/kernel/trace/trace_events_filter.c
+++ b/kernel/trace/trace_events_filter.c
@@ -2075,7 +2075,7 @@ struct function_filter_data {
 static char **
 ftrace_function_filter_re(char *buf, int len, int *count)
 {
-	char *str, *sep, **re;
+	char *str, **re;
 
 	str = kstrndup(buf, len, GFP_KERNEL);
 	if (!str)
@@ -2085,8 +2085,7 @@ ftrace_function_filter_re(char *buf, int len, int *count)
 	 * The argv_split function takes white space
 	 * as a separator, so convert ',' into spaces.
 	 */
-	while ((sep = strchr(str, ',')))
-		*sep = ' ';
+	strreplace(str, ',', ' ');
 
 	re = argv_split(GFP_KERNEL, str, count);
 	kfree(str);
-- 
2.1.3

Re: [RFC/PATCH 2/8] kernel/trace/trace_events_filter.c: Use strreplace

[Steven Rostedt]

On Thu,  4 Jun 2015 11:37:09 +0200
Rasmus Villemoes <linux@rasmusvillemoes.dk> wrote:

> There's no point in starting over every time we see a ','...
> 
> Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
> ---
>  kernel/trace/trace_events_filter.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/kernel/trace/trace_events_filter.c b/kernel/trace/trace_events_filter.c
> index ced69da0ff55..a987601d7b43 100644
> --- a/kernel/trace/trace_events_filter.c
> +++ b/kernel/trace/trace_events_filter.c
> @@ -2075,7 +2075,7 @@ struct function_filter_data {
>  static char **
>  ftrace_function_filter_re(char *buf, int len, int *count)
>  {
> -	char *str, *sep, **re;
> +	char *str, **re;
>  
>  	str = kstrndup(buf, len, GFP_KERNEL);
>  	if (!str)
> @@ -2085,8 +2085,7 @@ ftrace_function_filter_re(char *buf, int len, int *count)
>  	 * The argv_split function takes white space
>  	 * as a separator, so convert ',' into spaces.
>  	 */
> -	while ((sep = strchr(str, ',')))
> -		*sep = ' ';
> +	strreplace(str, ',', ' ');

Acked-by: Steven Rostedt <rostedt@goodmis.org>

-- Steve

>  
>  	re = argv_split(GFP_KERNEL, str, count);
>  	kfree(str);

[RFC/PATCH 3/8] blktrace: use strreplace in do_blk_trace_setup

[Rasmus Villemoes]

Part of the disassembly of do_blk_trace_setup:

    231b:       e8 00 00 00 00          callq  2320 <do_blk_trace_setup+0x50>
                        231c: R_X86_64_PC32     strlen+0xfffffffffffffffc
    2320:       eb 0a                   jmp    232c <do_blk_trace_setup+0x5c>
    2322:       66 0f 1f 44 00 00       nopw   0x0(%rax,%rax,1)
    2328:       48 83 c3 01             add    $0x1,%rbx
    232c:       48 39 d8                cmp    %rbx,%rax
    232f:       76 47                   jbe    2378 <do_blk_trace_setup+0xa8>
    2331:       41 80 3c 1c 2f          cmpb   $0x2f,(%r12,%rbx,1)
    2336:       75 f0                   jne    2328 <do_blk_trace_setup+0x58>
    2338:       41 c6 04 1c 5f          movb   $0x5f,(%r12,%rbx,1)
    233d:       4c 89 e7                mov    %r12,%rdi
    2340:       e8 00 00 00 00          callq  2345 <do_blk_trace_setup+0x75>
                        2341: R_X86_64_PC32     strlen+0xfffffffffffffffc
    2345:       eb e1                   jmp    2328 <do_blk_trace_setup+0x58>

Yep, that's right: gcc isn't smart enough to realize that replacing
'/' by '_' cannot change the strlen(), so we call it again and again
(at least when a '/' is found). Even if gcc were that smart, this
construction would still loop over the string twice, once for the
initial strlen() call and then the open-coded loop.

Let's simply use strreplace() instead.

Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
 kernel/trace/blktrace.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/kernel/trace/blktrace.c b/kernel/trace/blktrace.c
index 483cecfa5c17..4eeae4674b5a 100644
--- a/kernel/trace/blktrace.c
+++ b/kernel/trace/blktrace.c
@@ -439,7 +439,7 @@ int do_blk_trace_setup(struct request_queue *q, char *name, dev_t dev,
 {
 	struct blk_trace *old_bt, *bt = NULL;
 	struct dentry *dir = NULL;
-	int ret, i;
+	int ret;
 
 	if (!buts->buf_size || !buts->buf_nr)
 		return -EINVAL;
@@ -451,9 +451,7 @@ int do_blk_trace_setup(struct request_queue *q, char *name, dev_t dev,
 	 * some device names have larger paths - convert the slashes
 	 * to underscores for this to work as expected
 	 */
-	for (i = 0; i < strlen(buts->name); i++)
-		if (buts->name[i] == '/')
-			buts->name[i] = '_';
+	strreplace(buts->name, '/', '_');
 
 	bt = kzalloc(sizeof(*bt), GFP_KERNEL);
 	if (!bt)
-- 
2.1.3

Re: [RFC/PATCH 3/8] blktrace: use strreplace in do_blk_trace_setup

[Steven Rostedt]

blktrace is maintained by Jens Axboe (added to the Cc), although I also
maintain it for cleanups such as this. That's the reason it doesn't
have a "trace_" in its name like the other files do (a bit of trivia for
you ;-)

kernel/trace/blktrace.c should probably be included in the MAINTAINERS
file under BLOCK LAYER.


On Thu,  4 Jun 2015 11:37:10 +0200
Rasmus Villemoes <linux@rasmusvillemoes.dk> wrote:

> Part of the disassembly of do_blk_trace_setup:
> 
>     231b:       e8 00 00 00 00          callq  2320 <do_blk_trace_setup+0x50>
>                         231c: R_X86_64_PC32     strlen+0xfffffffffffffffc
>     2320:       eb 0a                   jmp    232c <do_blk_trace_setup+0x5c>
>     2322:       66 0f 1f 44 00 00       nopw   0x0(%rax,%rax,1)
>     2328:       48 83 c3 01             add    $0x1,%rbx
>     232c:       48 39 d8                cmp    %rbx,%rax
>     232f:       76 47                   jbe    2378 <do_blk_trace_setup+0xa8>
>     2331:       41 80 3c 1c 2f          cmpb   $0x2f,(%r12,%rbx,1)
>     2336:       75 f0                   jne    2328 <do_blk_trace_setup+0x58>
>     2338:       41 c6 04 1c 5f          movb   $0x5f,(%r12,%rbx,1)
>     233d:       4c 89 e7                mov    %r12,%rdi
>     2340:       e8 00 00 00 00          callq  2345 <do_blk_trace_setup+0x75>
>                         2341: R_X86_64_PC32     strlen+0xfffffffffffffffc
>     2345:       eb e1                   jmp    2328 <do_blk_trace_setup+0x58>
> 
> Yep, that's right: gcc isn't smart enough to realize that replacing
> '/' by '_' cannot change the strlen(), so we call it again and again
> (at least when a '/' is found). Even if gcc were that smart, this
> construction would still loop over the string twice, once for the
> initial strlen() call and then the open-coded loop.
> 
> Let's simply use strreplace() instead.
> 
> Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
> ---
>  kernel/trace/blktrace.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/kernel/trace/blktrace.c b/kernel/trace/blktrace.c
> index 483cecfa5c17..4eeae4674b5a 100644
> --- a/kernel/trace/blktrace.c
> +++ b/kernel/trace/blktrace.c
> @@ -439,7 +439,7 @@ int do_blk_trace_setup(struct request_queue *q, char *name, dev_t dev,
>  {
>  	struct blk_trace *old_bt, *bt = NULL;
>  	struct dentry *dir = NULL;
> -	int ret, i;
> +	int ret;
>  
>  	if (!buts->buf_size || !buts->buf_nr)
>  		return -EINVAL;
> @@ -451,9 +451,7 @@ int do_blk_trace_setup(struct request_queue *q, char *name, dev_t dev,
>  	 * some device names have larger paths - convert the slashes
>  	 * to underscores for this to work as expected
>  	 */
> -	for (i = 0; i < strlen(buts->name); i++)

Wow! I'm surprised this wasn't:

	for (i = 0; buts->name[i]; i++)

> -		if (buts->name[i] == '/')
> -			buts->name[i] = '_';
> +	strreplace(buts->name, '/', '_');
>  
>  	bt = kzalloc(sizeof(*bt), GFP_KERNEL);
>  	if (!bt)

Acked-by: Steven Rostedt <rostedt@goodmis.org>

-- Steve

[RFC/PATCH 4/8] lib/kobject.c: Use strreplace

[Rasmus Villemoes]

There's probably not many slashes in kobj->name, but starting over
when we see one feels wrong.

Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
 lib/kobject.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/lib/kobject.c b/lib/kobject.c
index 3b841b97fccd..597d962d3d4d 100644
--- a/lib/kobject.c
+++ b/lib/kobject.c
@@ -258,7 +258,6 @@ int kobject_set_name_vargs(struct kobject *kobj, const char *fmt,
 				  va_list vargs)
 {
 	const char *old_name = kobj->name;
-	char *s;
 
 	if (kobj->name && !fmt)
 		return 0;
@@ -270,8 +269,7 @@ int kobject_set_name_vargs(struct kobject *kobj, const char *fmt,
 	}
 
 	/* ewww... some of these buggers have '/' in the name ... */
-	while ((s = strchr(kobj->name, '/')))
-		s[0] = '!';
+	strreplace((char *)kobj->name, '/', '!');
 
 	kfree(old_name);
 	return 0;
-- 
2.1.3

Re: [RFC/PATCH 4/8] lib/kobject.c: Use strreplace

[Greg Kroah-Hartman]

On Thu, Jun 04, 2015 at 11:37:11AM +0200, Rasmus Villemoes wrote:
> There's probably not many slashes in kobj->name, but starting over
> when we see one feels wrong.
> 
> Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
> ---
>  lib/kobject.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)

Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Re: [RFC/PATCH 4/8] lib/kobject.c: Use strreplace

[Al Viro]

On Thu, Jun 04, 2015 at 11:37:11AM +0200, Rasmus Villemoes wrote:
> There's probably not many slashes in kobj->name, but starting over
> when we see one feels wrong.
> 
> Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
> ---
>  lib/kobject.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/lib/kobject.c b/lib/kobject.c
> index 3b841b97fccd..597d962d3d4d 100644
> --- a/lib/kobject.c
> +++ b/lib/kobject.c
> @@ -258,7 +258,6 @@ int kobject_set_name_vargs(struct kobject *kobj, const char *fmt,
>  				  va_list vargs)
>  {
>  	const char *old_name = kobj->name;
> -	char *s;
>  
>  	if (kobj->name && !fmt)
>  		return 0;
> @@ -270,8 +269,7 @@ int kobject_set_name_vargs(struct kobject *kobj, const char *fmt,
>  	}
>  
>  	/* ewww... some of these buggers have '/' in the name ... */
> -	while ((s = strchr(kobj->name, '/')))
> -		s[0] = '!';
> +	strreplace((char *)kobj->name, '/', '!');

Again, I'd rather do
        s = kvasprintf(GFP_KERNEL, fmt, vargs);
        if (!s)
                return -ENOMEM;
	strreplace(s, '/', '!');
	old_name = kobj->name;
	kobj->name = s;
	kfree(old_name);

[RFC/PATCH 5/8] drivers/base/core.c: Use strreplace

[Rasmus Villemoes]

This eliminates a local variable and a little .text.

Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
 drivers/base/core.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/base/core.c b/drivers/base/core.c
index 21d13038534e..01f2c1214f06 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -1282,8 +1282,6 @@ const char *device_get_devnode(struct device *dev,
 			       umode_t *mode, kuid_t *uid, kgid_t *gid,
 			       const char **tmp)
 {
-	char *s;
-
 	*tmp = NULL;
 
 	/* the device type may provide a specific name */
@@ -1306,8 +1304,7 @@ const char *device_get_devnode(struct device *dev,
 	*tmp = kstrdup(dev_name(dev), GFP_KERNEL);
 	if (!*tmp)
 		return NULL;
-	while ((s = strchr(*tmp, '!')))
-		s[0] = '/';
+	strreplace((char *)*tmp, '!', '/');
 	return *tmp;
 }
 
-- 
2.1.3

Re: [RFC/PATCH 5/8] drivers/base/core.c: Use strreplace

[Greg Kroah-Hartman]

On Thu, Jun 04, 2015 at 11:37:12AM +0200, Rasmus Villemoes wrote:
> This eliminates a local variable and a little .text.
> 
> Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
> ---
>  drivers/base/core.c | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
> 

Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Re: [RFC/PATCH 5/8] drivers/base/core.c: Use strreplace

[Al Viro]

On Thu, Jun 04, 2015 at 11:37:12AM +0200, Rasmus Villemoes wrote:
> This eliminates a local variable and a little .text.

Hmm...

>  	*tmp = kstrdup(dev_name(dev), GFP_KERNEL);
>  	if (!*tmp)
>  		return NULL;
> -	while ((s = strchr(*tmp, '!')))
> -		s[0] = '/';
> +	strreplace((char *)*tmp, '!', '/');
>  	return *tmp;

That cast isn't nice.  Why not make it
	s = kstrdup(...)
	if (s)
		strreplace(s, ...)
	return *tmp = s;
instead?

[RFC/PATCH 6/8] drivers/md/md.c: Use strreplace

[Rasmus Villemoes]

There's no point in starting over when we meet a '/'. This also
eliminates a stack variable and a little .text.

Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
 drivers/md/md.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/md/md.c b/drivers/md/md.c
index 27506302eb7a..2ea2f28551c5 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -2024,7 +2024,6 @@ static int bind_rdev_to_array(struct md_rdev *rdev, struct mddev *mddev)
 {
 	char b[BDEVNAME_SIZE];
 	struct kobject *ko;
-	char *s;
 	int err;
 
 	/* prevent duplicates */
@@ -2070,8 +2069,7 @@ static int bind_rdev_to_array(struct md_rdev *rdev, struct mddev *mddev)
 		return -EBUSY;
 	}
 	bdevname(rdev->bdev,b);
-	while ( (s=strchr(b, '/')) != NULL)
-		*s = '!';
+	strreplace(b, '/', '!');
 
 	rdev->mddev = mddev;
 	printk(KERN_INFO "md: bind<%s>\n", b);
-- 
2.1.3

[RFC/PATCH 7/8] fs/jbd2/journal.c: Use strreplace

[Rasmus Villemoes]

In one case, we eliminate a local variable; in the other a strlen()
call and some .text.

Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
 fs/jbd2/journal.c | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c
index b96bd8076b70..5c187ded12d6 100644
--- a/fs/jbd2/journal.c
+++ b/fs/jbd2/journal.c
@@ -1137,7 +1137,6 @@ journal_t * jbd2_journal_init_dev(struct block_device *bdev,
 {
 	journal_t *journal = journal_init_common();
 	struct buffer_head *bh;
-	char *p;
 	int n;
 
 	if (!journal)
@@ -1150,9 +1149,7 @@ journal_t * jbd2_journal_init_dev(struct block_device *bdev,
 	journal->j_blk_offset = start;
 	journal->j_maxlen = len;
 	bdevname(journal->j_dev, journal->j_devname);
-	p = journal->j_devname;
-	while ((p = strchr(p, '/')))
-		*p = '!';
+	strreplace(journal->j_devname, '/', '!');
 	jbd2_stats_proc_init(journal);
 	n = journal->j_blocksize / sizeof(journal_block_tag_t);
 	journal->j_wbufsize = n;
@@ -1204,10 +1201,7 @@ journal_t * jbd2_journal_init_inode (struct inode *inode)
 	journal->j_dev = journal->j_fs_dev = inode->i_sb->s_bdev;
 	journal->j_inode = inode;
 	bdevname(journal->j_dev, journal->j_devname);
-	p = journal->j_devname;
-	while ((p = strchr(p, '/')))
-		*p = '!';
-	p = journal->j_devname + strlen(journal->j_devname);
+	p = strreplace(journal->j_devname, '/', '!');
 	sprintf(p, "-%lu", journal->j_inode->i_ino);
 	jbd_debug(1,
 		  "journal %p: inode %s/%ld, size %Ld, bits %d, blksize %ld\n",
-- 
2.1.3

[RFC/PATCH 8/8] fs/ext4/super.c: Use strreplace in ext4_fill_super

[Rasmus Villemoes]

This makes a very large function a little smaller.

Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
 fs/ext4/super.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index ca9d4a2fed41..5f3c43a66937 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -3420,7 +3420,6 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent)
 	unsigned long journal_devnum = 0;
 	unsigned long def_mount_opts;
 	struct inode *root;
-	char *cp;
 	const char *descr;
 	int ret = -ENOMEM;
 	int blocksize, clustersize;
@@ -3456,8 +3455,7 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent)
 #endif
 
 	/* Cleanup superblock name */
-	for (cp = sb->s_id; (cp = strchr(cp, '/'));)
-		*cp = '!';
+	strreplace(sb->s_id, '/', '!');
 
 	/* -EINVAL is default */
 	ret = -EINVAL;
-- 
2.1.3