[PATCH v2 0/3] lib/string_helpers et al.: Change return value of strreplace()

[PATCH v2 0/3] lib/string_helpers et al.: Change return value of strreplace()

[Andy Shevchenko]

It's more convenient to have strreplace() to return the pointer to
 the string itself. This will help users to make their code better.

The patch 1 kills the only user of the returned value of strreplace(),
Patch 2 converts the return value of strreplace(). And patch 3 shows
how it may be useful. That said, the series can be routed via fs tree,
with or without the last patch.

In v2:
- removed not anymore used variable (LKP)
- added tag (Jan)
- fixed wording (Kees)
- actually return the pointer to the string itself

Andy Shevchenko (3):
  jbd2: Avoid printing outside the boundary of the buffer
  lib/string_helpers: Change returned value of the strreplace()
  kobject: Use return value of strreplace()

 fs/jbd2/journal.c      |  6 ++----
 include/linux/string.h |  2 +-
 lib/kobject.c          |  3 +--
 lib/string_helpers.c   | 12 ++++++++----
 4 files changed, 12 insertions(+), 11 deletions(-)

-- 
2.40.0.1.gaa8946217a0b

[PATCH v2 1/3] jbd2: Avoid printing outside the boundary of the buffer

[Andy Shevchenko]

Theoretically possible that "%pg" will take all room for the j_devname
and hence the "-%lu" will go outside the boundary due to unconditional
sprintf() in use. To make this code more robust, replace two sequential
s*printf():s by a single call and then replace forbidden character.
It's possible to do this way, because '/' won't ever be in the result
of "-%lu".

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-by: Jan Kara <jack@suse.cz>
---
 fs/jbd2/journal.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c
index 8ae419152ff6..6e17f8f94dfd 100644
--- a/fs/jbd2/journal.c
+++ b/fs/jbd2/journal.c
@@ -1491,7 +1491,6 @@ journal_t *jbd2_journal_init_inode(struct inode *inode)
 {
 	journal_t *journal;
 	sector_t blocknr;
-	char *p;
 	int err = 0;
 
 	blocknr = 0;
@@ -1515,9 +1514,8 @@ journal_t *jbd2_journal_init_inode(struct inode *inode)
 
 	journal->j_inode = inode;
 	snprintf(journal->j_devname, sizeof(journal->j_devname),
-		 "%pg", journal->j_dev);
-	p = strreplace(journal->j_devname, '/', '!');
-	sprintf(p, "-%lu", journal->j_inode->i_ino);
+		 "%pg-%lu", journal->j_dev, journal->j_inode->i_ino);
+	strreplace(journal->j_devname, '/', '!');
 	jbd2_stats_proc_init(journal);
 
 	return journal;
-- 
2.40.0.1.gaa8946217a0b

[PATCH v2 2/3] lib/string_helpers: Change returned value of the strreplace()

[Andy Shevchenko]

It's more useful to return the pointer to the string itself
with strreplace(), so it may be used like

	attr->name = strreplace(name, '/', '_');

While at it, amend the kernel documentation.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 include/linux/string.h |  2 +-
 lib/string_helpers.c   | 12 ++++++++----
 2 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/include/linux/string.h b/include/linux/string.h
index a7bff7ed3cb0..cb0c24ce0826 100644
--- a/include/linux/string.h
+++ b/include/linux/string.h
@@ -169,7 +169,7 @@ static inline void memcpy_flushcache(void *dst, const void *src, size_t cnt)
 #endif
 
 void *memchr_inv(const void *s, int c, size_t n);
-char *strreplace(char *s, char old, char new);
+char *strreplace(char *str, char old, char new);
 
 extern void kfree_const(const void *x);
 
diff --git a/lib/string_helpers.c b/lib/string_helpers.c
index 230020a2e076..d3b1dd718daf 100644
--- a/lib/string_helpers.c
+++ b/lib/string_helpers.c
@@ -979,18 +979,22 @@ EXPORT_SYMBOL(__sysfs_match_string);
 
 /**
  * strreplace - Replace all occurrences of character in string.
- * @s: The string to operate on.
+ * @str: 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.
+ * Replaces the each @old character with a @new one in the given string @str.
+ *
+ * Return: pointer to the string @str itself.
  */
-char *strreplace(char *s, char old, char new)
+char *strreplace(char *str, char old, char new)
 {
+	char *s = str;
+
 	for (; *s; ++s)
 		if (*s == old)
 			*s = new;
-	return s;
+	return str;
 }
 EXPORT_SYMBOL(strreplace);
 
-- 
2.40.0.1.gaa8946217a0b

[PATCH v2 3/3] kobject: Use return value of strreplace()

[Andy Shevchenko]

Since strreplace() returns the pointer to the string itself,
we may use it directly in the code.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 lib/kobject.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/lib/kobject.c b/lib/kobject.c
index f79a434e1231..16d530f9c174 100644
--- a/lib/kobject.c
+++ b/lib/kobject.c
@@ -281,8 +281,7 @@ int kobject_set_name_vargs(struct kobject *kobj, const char *fmt,
 		kfree_const(s);
 		if (!t)
 			return -ENOMEM;
-		strreplace(t, '/', '!');
-		s = t;
+		s = strreplace(t, '/', '!');
 	}
 	kfree_const(kobj->name);
 	kobj->name = s;
-- 
2.40.0.1.gaa8946217a0b

RE: [PATCH v2 3/3] kobject: Use return value of strreplace()

[David Laight]

From: Andy Shevchenko
> Sent: 23 March 2023 12:37
>
> Since strreplace() returns the pointer to the string itself,
> we may use it directly in the code.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  lib/kobject.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/lib/kobject.c b/lib/kobject.c
> index f79a434e1231..16d530f9c174 100644
> --- a/lib/kobject.c
> +++ b/lib/kobject.c
> @@ -281,8 +281,7 @@ int kobject_set_name_vargs(struct kobject *kobj, const char *fmt,
>  		kfree_const(s);
>  		if (!t)
>  			return -ENOMEM;
> -		strreplace(t, '/', '!');
> -		s = t;
> +		s = strreplace(t, '/', '!');

Why do this? It just makes the code harder to read because
you have to know another 'silly fact' about a function.

Possibly useful return values might be:
1) The address of the first changed character.
2) The address of the last changed characher.
3) The '\0' terminator.
4) void.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

Re: [PATCH v2 0/3] lib/string_helpers et al.: Change return value of strreplace()

[Andy Shevchenko]

On Thu, Mar 23, 2023 at 02:37:01PM +0200, Andy Shevchenko wrote:
> It's more convenient to have strreplace() to return the pointer to
>  the string itself. This will help users to make their code better.
> 
> The patch 1 kills the only user of the returned value of strreplace(),
> Patch 2 converts the return value of strreplace(). And patch 3 shows
> how it may be useful. That said, the series can be routed via fs tree,
> with or without the last patch.

Since there are no comments, who can apply this (patches 1 and 2)?
Greg, are you fine with the kobject change?

> In v2:
> - removed not anymore used variable (LKP)
> - added tag (Jan)
> - fixed wording (Kees)
> - actually return the pointer to the string itself
> 
> Andy Shevchenko (3):
>   jbd2: Avoid printing outside the boundary of the buffer
>   lib/string_helpers: Change returned value of the strreplace()
>   kobject: Use return value of strreplace()
> 
>  fs/jbd2/journal.c      |  6 ++----
>  include/linux/string.h |  2 +-
>  lib/kobject.c          |  3 +--
>  lib/string_helpers.c   | 12 ++++++++----
>  4 files changed, 12 insertions(+), 11 deletions(-)

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 0/3] lib/string_helpers et al.: Change return value of strreplace()

[Greg Kroah-Hartman]

On Wed, Apr 05, 2023 at 04:34:31PM +0300, Andy Shevchenko wrote:
> On Thu, Mar 23, 2023 at 02:37:01PM +0200, Andy Shevchenko wrote:
> > It's more convenient to have strreplace() to return the pointer to
> >  the string itself. This will help users to make their code better.
> > 
> > The patch 1 kills the only user of the returned value of strreplace(),
> > Patch 2 converts the return value of strreplace(). And patch 3 shows
> > how it may be useful. That said, the series can be routed via fs tree,
> > with or without the last patch.
> 
> Since there are no comments, who can apply this (patches 1 and 2)?
> Greg, are you fine with the kobject change?

Sure, want me to take them all through my driver-core tree?

thanks,

greg k-h

Re: [PATCH v2 0/3] lib/string_helpers et al.: Change return value of strreplace()

[Andy Shevchenko]

On Wed, Apr 05, 2023 at 04:24:42PM +0200, Greg Kroah-Hartman wrote:
> On Wed, Apr 05, 2023 at 04:34:31PM +0300, Andy Shevchenko wrote:
> > On Thu, Mar 23, 2023 at 02:37:01PM +0200, Andy Shevchenko wrote:
> > > It's more convenient to have strreplace() to return the pointer to
> > >  the string itself. This will help users to make their code better.
> > > 
> > > The patch 1 kills the only user of the returned value of strreplace(),
> > > Patch 2 converts the return value of strreplace(). And patch 3 shows
> > > how it may be useful. That said, the series can be routed via fs tree,
> > > with or without the last patch.
> > 
> > Since there are no comments, who can apply this (patches 1 and 2)?
> > Greg, are you fine with the kobject change?
> 
> Sure, want me to take them all through my driver-core tree?

Fine by me! Dunno about others. Kees?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 0/3] lib/string_helpers et al.: Change return value of strreplace()

[Kees Cook]

On April 5, 2023 7:38:40 AM PDT, Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
>On Wed, Apr 05, 2023 at 04:24:42PM +0200, Greg Kroah-Hartman wrote:
>> On Wed, Apr 05, 2023 at 04:34:31PM +0300, Andy Shevchenko wrote:
>> > On Thu, Mar 23, 2023 at 02:37:01PM +0200, Andy Shevchenko wrote:
>> > > It's more convenient to have strreplace() to return the pointer to
>> > >  the string itself. This will help users to make their code better.
>> > > 
>> > > The patch 1 kills the only user of the returned value of strreplace(),
>> > > Patch 2 converts the return value of strreplace(). And patch 3 shows
>> > > how it may be useful. That said, the series can be routed via fs tree,
>> > > with or without the last patch.
>> > 
>> > Since there are no comments, who can apply this (patches 1 and 2)?
>> > Greg, are you fine with the kobject change?
>> 
>> Sure, want me to take them all through my driver-core tree?
>
>Fine by me! Dunno about others. Kees?

Yeah, that's cool by me. :)


-- 
Kees Cook

Re: [PATCH v2 0/3] lib/string_helpers et al.: Change return value of strreplace()

[Andy Shevchenko]

On Wed, Apr 05, 2023 at 07:58:40PM -0700, Kees Cook wrote:
> On April 5, 2023 7:38:40 AM PDT, Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> >On Wed, Apr 05, 2023 at 04:24:42PM +0200, Greg Kroah-Hartman wrote:
> >> On Wed, Apr 05, 2023 at 04:34:31PM +0300, Andy Shevchenko wrote:
> >> > On Thu, Mar 23, 2023 at 02:37:01PM +0200, Andy Shevchenko wrote:
> >> > > It's more convenient to have strreplace() to return the pointer to
> >> > >  the string itself. This will help users to make their code better.
> >> > > 
> >> > > The patch 1 kills the only user of the returned value of strreplace(),
> >> > > Patch 2 converts the return value of strreplace(). And patch 3 shows
> >> > > how it may be useful. That said, the series can be routed via fs tree,
> >> > > with or without the last patch.
> >> > 
> >> > Since there are no comments, who can apply this (patches 1 and 2)?
> >> > Greg, are you fine with the kobject change?
> >> 
> >> Sure, want me to take them all through my driver-core tree?
> >
> >Fine by me! Dunno about others. Kees?
> 
> Yeah, that's cool by me. :)

Greg, does this slip through the cracks?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 0/3] lib/string_helpers et al.: Change return value of strreplace()

[Greg Kroah-Hartman]

On Mon, Jun 05, 2023 at 05:04:43PM +0300, Andy Shevchenko wrote:
> On Wed, Apr 05, 2023 at 07:58:40PM -0700, Kees Cook wrote:
> > On April 5, 2023 7:38:40 AM PDT, Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> > >On Wed, Apr 05, 2023 at 04:24:42PM +0200, Greg Kroah-Hartman wrote:
> > >> On Wed, Apr 05, 2023 at 04:34:31PM +0300, Andy Shevchenko wrote:
> > >> > On Thu, Mar 23, 2023 at 02:37:01PM +0200, Andy Shevchenko wrote:
> > >> > > It's more convenient to have strreplace() to return the pointer to
> > >> > >  the string itself. This will help users to make their code better.
> > >> > > 
> > >> > > The patch 1 kills the only user of the returned value of strreplace(),
> > >> > > Patch 2 converts the return value of strreplace(). And patch 3 shows
> > >> > > how it may be useful. That said, the series can be routed via fs tree,
> > >> > > with or without the last patch.
> > >> > 
> > >> > Since there are no comments, who can apply this (patches 1 and 2)?
> > >> > Greg, are you fine with the kobject change?
> > >> 
> > >> Sure, want me to take them all through my driver-core tree?
> > >
> > >Fine by me! Dunno about others. Kees?
> > 
> > Yeah, that's cool by me. :)
> 
> Greg, does this slip through the cracks?

It did.  Can someone resend this?

thanks,

greg k-h

Re: [PATCH v2 0/3] lib/string_helpers et al.: Change return value of strreplace()

[Andy Shevchenko]

On Mon, Jun 05, 2023 at 06:57:48PM +0200, Greg Kroah-Hartman wrote:
> On Mon, Jun 05, 2023 at 05:04:43PM +0300, Andy Shevchenko wrote:
> > On Wed, Apr 05, 2023 at 07:58:40PM -0700, Kees Cook wrote:
> > > On April 5, 2023 7:38:40 AM PDT, Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> > > >On Wed, Apr 05, 2023 at 04:24:42PM +0200, Greg Kroah-Hartman wrote:
> > > >> On Wed, Apr 05, 2023 at 04:34:31PM +0300, Andy Shevchenko wrote:
> > > >> > On Thu, Mar 23, 2023 at 02:37:01PM +0200, Andy Shevchenko wrote:
> > > >> > > It's more convenient to have strreplace() to return the pointer to
> > > >> > >  the string itself. This will help users to make their code better.
> > > >> > > 
> > > >> > > The patch 1 kills the only user of the returned value of strreplace(),
> > > >> > > Patch 2 converts the return value of strreplace(). And patch 3 shows
> > > >> > > how it may be useful. That said, the series can be routed via fs tree,
> > > >> > > with or without the last patch.
> > > >> > 
> > > >> > Since there are no comments, who can apply this (patches 1 and 2)?
> > > >> > Greg, are you fine with the kobject change?
> > > >> 
> > > >> Sure, want me to take them all through my driver-core tree?
> > > >
> > > >Fine by me! Dunno about others. Kees?
> > > 
> > > Yeah, that's cool by me. :)
> > 
> > Greg, does this slip through the cracks?
> 
> It did.  Can someone resend this?

Done as v3.
20230605170553.7835-1-andriy.shevchenko@linux.intel.com

-- 
With Best Regards,
Andy Shevchenko