[PATCH v2 0/8] Introduce strreplace

classic Classic list List threaded Threaded
16 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH v2 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.

Andrew, can I get you to take 1/8 through the mm tree? I'm not sure
what the easiest path is for the remaining patches.

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                |  9 ++++-----
 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                      | 13 +++++--------
 lib/string.c                       | 17 +++++++++++++++++
 9 files changed, 35 insertions(+), 34 deletions(-)

--
2.1.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
Reply | Threaded
Open this post in threaded view
|

[PATCH v2 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 <[hidden email]>
Acked-by: Steven Rostedt <[hidden email]>
---
v2: same patch, added Ack.

 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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
Reply | Threaded
Open this post in threaded view
|

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

Rasmus Villemoes
In reply to this post by Rasmus Villemoes
This eliminates a little .text and avoids repeating the strchr call
when we meet a '!' (which will happen at least once).

Signed-off-by: Rasmus Villemoes <[hidden email]>
---
v2: Avoid ugly (char *) cast.

 drivers/base/core.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/base/core.c b/drivers/base/core.c
index 21d13038534e..dafae6d2f7ac 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -1303,12 +1303,11 @@ const char *device_get_devnode(struct device *dev,
  return dev_name(dev);
 
  /* replace '!' in the name with '/' */
- *tmp = kstrdup(dev_name(dev), GFP_KERNEL);
- if (!*tmp)
+ s = kstrdup(dev_name(dev), GFP_KERNEL);
+ if (!s)
  return NULL;
- while ((s = strchr(*tmp, '!')))
- s[0] = '/';
- return *tmp;
+ strreplace(s, '!', '/');
+ return *tmp = s;
 }
 
 /**
--
2.1.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
Reply | Threaded
Open this post in threaded view
|

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

Rasmus Villemoes
In reply to this post by Rasmus Villemoes
In one case, we eliminate a local variable; in the other a strlen()
call and some .text.

Signed-off-by: Rasmus Villemoes <[hidden email]>
---
v2: no changes.

 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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
Reply | Threaded
Open this post in threaded view
|

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

Rasmus Villemoes
In reply to this post by 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 <[hidden email]>
---
v2: no changes.

 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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
Reply | Threaded
Open this post in threaded view
|

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

Rasmus Villemoes
In reply to this post by Rasmus Villemoes
This makes a very large function a little smaller.

Signed-off-by: Rasmus Villemoes <[hidden email]>
---
v2: no changes.

 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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
Reply | Threaded
Open this post in threaded view
|

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

Rasmus Villemoes
In reply to this post by Rasmus Villemoes
There's probably not many slashes in the name, but starting over when
we see one feels wrong.

Signed-off-by: Rasmus Villemoes <[hidden email]>
---
v2: Original code relied on the const laundering done by strchr; v1
had a corresponding explicit (char*) cast. Avoid this altogether.

 lib/kobject.c | 13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/lib/kobject.c b/lib/kobject.c
index 3b841b97fccd..75ee63834fd1 100644
--- a/lib/kobject.c
+++ b/lib/kobject.c
@@ -257,23 +257,20 @@ static int kobject_add_internal(struct kobject *kobj)
 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;
 
- kobj->name = kvasprintf(GFP_KERNEL, fmt, vargs);
- if (!kobj->name) {
- kobj->name = old_name;
+ s = kvasprintf(GFP_KERNEL, fmt, vargs);
+ if (!s)
  return -ENOMEM;
- }
 
  /* ewww... some of these buggers have '/' in the name ... */
- while ((s = strchr(kobj->name, '/')))
- s[0] = '!';
+ strreplace(s, '/', '!');
+ kfree(kobj->name);
+ kobj->name = s;
 
- kfree(old_name);
  return 0;
 }
 
--
2.1.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
Reply | Threaded
Open this post in threaded view
|

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

Rasmus Villemoes
In reply to this post by 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 <[hidden email]>
Acked-by: Steven Rostedt <[hidden email]>
---
v2: same patch, added Ack.

 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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
Reply | Threaded
Open this post in threaded view
|

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

Rasmus Villemoes
In reply to this post by 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 <[hidden email]>
---
v2: spello fixed, parameters renamed 'old' and 'new' (just so the
kernel doc aligns nicely, and because that's what python -c
'help(str.replace)' uses). Still EXPORT_SYMBOL, not inline (tried it,
caused more bloat), still called strreplace.

 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..a8d90db9c4b0 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);
+char *strreplace(char *s, char old, char new);
 
 extern void kfree_const(const void *x);
 
diff --git a/lib/string.c b/lib/string.c
index bb3d4b6993c4..13d1e84ddb80 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 occurrences of character in string.
+ * @s: The string to operate on.
+ * @old: The character being replaced.
+ * @new: The character @old is replaced with.
+ *
+ * Returns pointer to the nul byte at the end of @s.
+ */
+char *strreplace(char *s, char old, char new)
+{
+ for (; *s; ++s)
+ if (*s == old)
+ *s = new;
+ return s;
+}
+EXPORT_SYMBOL(strreplace);
--
2.1.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
Reply | Threaded
Open this post in threaded view
|

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

Joe Perches
On Tue, 2015-06-09 at 01:26 +0200, Rasmus Villemoes wrote:
> Strings are sometimes sanitized by replacing a certain character
> (often '/') by another (often '!').
[]
> v2: spello fixed, parameters renamed 'old' and 'new' (just so the
> kernel doc aligns nicely, and because that's what python -c
> 'help(str.replace)' uses). Still EXPORT_SYMBOL, not inline (tried it,
> caused more bloat), still called strreplace.

OK, thanks.  I think the chars should be ints though
just for consistency for strchr variants.

> diff --git 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);
> +char *strreplace(char *s, char old, char new);


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v2 0/8] Introduce strreplace

Theodore Ts'o
In reply to this post by Rasmus Villemoes
On Tue, Jun 09, 2015 at 01:26:48AM +0200, Rasmus Villemoes wrote:
> 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.
>
> Andrew, can I get you to take 1/8 through the mm tree? I'm not sure
> what the easiest path is for the remaining patches.

This is not super urgent, right?  So we could let 1/8 go into
mainline, and then the rest of the patches could go in the next
release.  That would be the simplest, although it would drag out how
long it would take for strreplace to be used everywhere.

                          - Ted
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
Reply | Threaded
Open this post in threaded view
|

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

Jens Axboe-4
In reply to this post by Rasmus Villemoes
On 06/08/2015 05:26 PM, Rasmus Villemoes 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.

Patch looks fine to me, but there's no strreplace in in Linus' tree.
Dependencies like that should be noted in the patch. Please send
followup patches like this when the main patch is in Linus tree, and I'd
be happy to apply it.

--
Jens Axboe

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
Reply | Threaded
Open this post in threaded view
|

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

Rasmus Villemoes
In reply to this post by Joe Perches
On Tue, Jun 09 2015, Joe Perches <[hidden email]> wrote:

> On Tue, 2015-06-09 at 01:26 +0200, Rasmus Villemoes wrote:
>> Strings are sometimes sanitized by replacing a certain character
>> (often '/') by another (often '!').
> []
>> v2: spello fixed, parameters renamed 'old' and 'new' (just so the
>> kernel doc aligns nicely, and because that's what python -c
>> 'help(str.replace)' uses). Still EXPORT_SYMBOL, not inline (tried it,
>> caused more bloat), still called strreplace.
>
> OK, thanks.  I think the chars should be ints though
> just for consistency for strchr variants.

I disagree. That way lies subtle (semi)bugs. Quick quiz: What does
memscan return below?

  char a[1] = {X}; /* for some suitable X */
  char *p = memscan(a, a[0], 1);

Here's the kerneldoc+prototype for memscan:

/**
 * memscan(void *addr, int c, size_t size) - Find a character in an area of memory.
 * @addr: The memory area
 * @c: The byte to search for
 * @size: The size of the area.
 *
 * returns the address of the first occurrence of @c, or 1 byte past
 * the area if @c is not found
 */

So obviously p==a, right? Wrong. Or rather, wrong when char is signed
and X lies outside the ascii range. Or maybe right, if you're on an
architecture with its own memscan that DTRT. And 'the right thing' is
obviously to use only the LSB of c, which would have been harder to get
wrong if c was just a u8 to begin with.

(The only in-tree callers which do not pass an explicit non-negative
constant seem to be in drivers/hid/usbhid/usbkbd.c and
net/bluetooth/hidp/core.c, and they both pass something from an unsigned
char array).

We're stuck with int in the libc functions, but we can do better for new
interfaces.

Rasmus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v2 0/8] Introduce strreplace

Rasmus Villemoes
In reply to this post by Theodore Ts'o
On Tue, Jun 09 2015, Theodore Ts'o <[hidden email]> wrote:

> On Tue, Jun 09, 2015 at 01:26:48AM +0200, Rasmus Villemoes wrote:
>> 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.
>>
>> Andrew, can I get you to take 1/8 through the mm tree? I'm not sure
>> what the easiest path is for the remaining patches.
>
> This is not super urgent, right?

Right, not urgent at all.

> So we could let 1/8 go into mainline, and then the rest of the patches
> could go in the next release. That would be the simplest, although it
> would drag out how long it would take for strreplace to be used
> everywhere.

Sure, though it would be a little weird to have a helper with no users
until 4.3 comes out.

Rasmus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v2 0/8] Introduce strreplace

akpm
In reply to this post by Rasmus Villemoes
On Tue,  9 Jun 2015 01:26:48 +0200 Rasmus Villemoes <[hidden email]> wrote:

> 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.
>
> Andrew, can I get you to take 1/8 through the mm tree? I'm not sure
> what the easiest path is for the remaining patches.

With this sort of thing I grab everything them feed the dependent
patches to maintainers after the base patch is upstream.

Or I merge the dependent patches myself if they were acked.

Or if the dependent patches are simple I'll just merge them anyway,
shrug.  I'd say these fall into that category.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
Reply | Threaded
Open this post in threaded view
|

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

NeilBrown
In reply to this post by Rasmus Villemoes
On Tue,  9 Jun 2015 01:26:54 +0200
Rasmus Villemoes <[hidden email]> wrote:

> 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 <[hidden email]>
> ---
> v2: no changes.
>
>  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);


Acked-by: NeilBrown <[hidden email]>

I'm happy for Andrew to merge this.
Thanks,
NeilBrown
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/