[PATCH v2 01/10] string: provide strends()

Bartosz Golaszewski posted 10 patches 3 months, 2 weeks ago
There is a newer version of this series
[PATCH v2 01/10] string: provide strends()
Posted by Bartosz Golaszewski 3 months, 2 weeks ago
From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

Implement a function for checking if a string ends with a different
string and add its kunit test cases.

Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
 include/linux/string.h   |  2 ++
 lib/string.c             | 19 +++++++++++++++++++
 lib/tests/string_kunit.c | 13 +++++++++++++
 3 files changed, 34 insertions(+)

diff --git a/include/linux/string.h b/include/linux/string.h
index fdd3442c6bcbd786e177b6e87358e1065a0ffafc..16149404fc3aed535c094b9550f7bd7c9b44a0c9 100644
--- a/include/linux/string.h
+++ b/include/linux/string.h
@@ -562,4 +562,6 @@ static inline bool strstarts(const char *str, const char *prefix)
 	return strncmp(str, prefix, strlen(prefix)) == 0;
 }
 
+bool strends(const char *str, const char *suffix);
+
 #endif /* _LINUX_STRING_H_ */
diff --git a/lib/string.c b/lib/string.c
index b632c71df1a5061256f556c40a30587cf45dce18..ac18313d9c52b38132f0bfdd952cdec6b1354d76 100644
--- a/lib/string.c
+++ b/lib/string.c
@@ -880,3 +880,22 @@ void *memchr_inv(const void *start, int c, size_t bytes)
 	return check_bytes8(start, value, bytes % 8);
 }
 EXPORT_SYMBOL(memchr_inv);
+
+/**
+ * strends - Check if a string ends with another string.
+ * @str - NULL-terminated string to check against @suffix
+ * @suffix - NULL-terminated string defining the suffix to look for in @str
+ *
+ * Returns:
+ * True if @str ends with @suffix. False in all other cases.
+ */
+bool strends(const char *str, const char *suffix)
+{
+	unsigned int str_len = strlen(str), suffix_len = strlen(suffix);
+
+	if (str_len < suffix_len)
+		return false;
+
+	return !(strcmp(str + str_len - suffix_len, suffix));
+}
+EXPORT_SYMBOL(strends);
diff --git a/lib/tests/string_kunit.c b/lib/tests/string_kunit.c
index 0ed7448a26d3aa0fe9e2a6a894d4c49c2c0b86e0..f9a8e557ba7734c9848d58ff986407d8000f52ee 100644
--- a/lib/tests/string_kunit.c
+++ b/lib/tests/string_kunit.c
@@ -602,6 +602,18 @@ static void string_test_memtostr(struct kunit *test)
 	KUNIT_EXPECT_EQ(test, dest[7], '\0');
 }
 
+static void string_test_strends(struct kunit *test)
+{
+	KUNIT_EXPECT_TRUE(test, strends("foo-bar", "bar"));
+	KUNIT_EXPECT_TRUE(test, strends("foo-bar", "-bar"));
+	KUNIT_EXPECT_TRUE(test, strends("foobar", "foobar"));
+	KUNIT_EXPECT_TRUE(test, strends("foobar", ""));
+	KUNIT_EXPECT_FALSE(test, strends("bar", "foobar"));
+	KUNIT_EXPECT_FALSE(test, strends("", "foo"));
+	KUNIT_EXPECT_FALSE(test, strends("foobar", "ba"));
+	KUNIT_EXPECT_TRUE(test, strends("", ""));
+}
+
 static struct kunit_case string_test_cases[] = {
 	KUNIT_CASE(string_test_memset16),
 	KUNIT_CASE(string_test_memset32),
@@ -623,6 +635,7 @@ static struct kunit_case string_test_cases[] = {
 	KUNIT_CASE(string_test_strlcat),
 	KUNIT_CASE(string_test_strtomem),
 	KUNIT_CASE(string_test_memtostr),
+	KUNIT_CASE(string_test_strends),
 	{}
 };
 

-- 
2.48.1
Re: [PATCH v2 01/10] string: provide strends()
Posted by Andy Shevchenko 3 months, 2 weeks ago
On Wed, Oct 22, 2025 at 03:10:40PM +0200, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> 
> Implement a function for checking if a string ends with a different
> string and add its kunit test cases.

...

> +static void string_test_strends(struct kunit *test)
> +{
> +	KUNIT_EXPECT_TRUE(test, strends("foo-bar", "bar"));
> +	KUNIT_EXPECT_TRUE(test, strends("foo-bar", "-bar"));
> +	KUNIT_EXPECT_TRUE(test, strends("foobar", "foobar"));
> +	KUNIT_EXPECT_TRUE(test, strends("foobar", ""));
> +	KUNIT_EXPECT_FALSE(test, strends("bar", "foobar"));
> +	KUNIT_EXPECT_FALSE(test, strends("", "foo"));
> +	KUNIT_EXPECT_FALSE(test, strends("foobar", "ba"));
> +	KUNIT_EXPECT_TRUE(test, strends("", ""));
> +}

Have you checked the binary file? If you want this to be properly implemented,
generate the suffix. (Actually making the function static inline makes my point
really visible)

-- 
With Best Regards,
Andy Shevchenko
Re: [PATCH v2 01/10] string: provide strends()
Posted by Bartosz Golaszewski 3 months, 2 weeks ago
On Wed, Oct 22, 2025 at 5:25 PM Andy Shevchenko
<andriy.shevchenko@intel.com> wrote:
>
> On Wed, Oct 22, 2025 at 03:10:40PM +0200, Bartosz Golaszewski wrote:
> > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> >
> > Implement a function for checking if a string ends with a different
> > string and add its kunit test cases.
>
> ...
>
> > +static void string_test_strends(struct kunit *test)
> > +{
> > +     KUNIT_EXPECT_TRUE(test, strends("foo-bar", "bar"));
> > +     KUNIT_EXPECT_TRUE(test, strends("foo-bar", "-bar"));
> > +     KUNIT_EXPECT_TRUE(test, strends("foobar", "foobar"));
> > +     KUNIT_EXPECT_TRUE(test, strends("foobar", ""));
> > +     KUNIT_EXPECT_FALSE(test, strends("bar", "foobar"));
> > +     KUNIT_EXPECT_FALSE(test, strends("", "foo"));
> > +     KUNIT_EXPECT_FALSE(test, strends("foobar", "ba"));
> > +     KUNIT_EXPECT_TRUE(test, strends("", ""));
> > +}
>
> Have you checked the binary file? If you want this to be properly implemented,
> generate the suffix. (Actually making the function static inline makes my point
> really visible)
>

Andy, this is bikeshedding. This is literally the least important
piece of this series. It doesn't matter for the big picture whether
this is inlined or not.

Bartosz
Re: [PATCH v2 01/10] string: provide strends()
Posted by Andy Shevchenko 3 months, 2 weeks ago
On Wed, Oct 22, 2025 at 05:36:33PM +0200, Bartosz Golaszewski wrote:
> On Wed, Oct 22, 2025 at 5:25 PM Andy Shevchenko
> <andriy.shevchenko@intel.com> wrote:
> > On Wed, Oct 22, 2025 at 03:10:40PM +0200, Bartosz Golaszewski wrote:

...

> > > +static void string_test_strends(struct kunit *test)
> > > +{
> > > +     KUNIT_EXPECT_TRUE(test, strends("foo-bar", "bar"));
> > > +     KUNIT_EXPECT_TRUE(test, strends("foo-bar", "-bar"));
> > > +     KUNIT_EXPECT_TRUE(test, strends("foobar", "foobar"));
> > > +     KUNIT_EXPECT_TRUE(test, strends("foobar", ""));
> > > +     KUNIT_EXPECT_FALSE(test, strends("bar", "foobar"));
> > > +     KUNIT_EXPECT_FALSE(test, strends("", "foo"));
> > > +     KUNIT_EXPECT_FALSE(test, strends("foobar", "ba"));
> > > +     KUNIT_EXPECT_TRUE(test, strends("", ""));
> > > +}
> >
> > Have you checked the binary file? If you want this to be properly implemented,
> > generate the suffix. (Actually making the function static inline makes my point
> > really visible)
> 
> Andy, this is bikeshedding. This is literally the least important
> piece of this series. It doesn't matter for the big picture whether
> this is inlined or not.

It's definitely not a bikeshedding. I try to keep a bit consistency here and
I don't see the point of bloating a kernel (binary as well) for the function
that just a couple of lines with simple basic calls.

Also note that with inlined version strlen() for string literals will be
calculated at _compile-time_! This is clear benefit.

Really, library code is not as simple as dropping something to somewhere...

-- 
With Best Regards,
Andy Shevchenko


Re: [PATCH v2 01/10] string: provide strends()
Posted by Bartosz Golaszewski 3 months, 2 weeks ago
On Wed, Oct 22, 2025 at 7:12 PM Andy Shevchenko
<andriy.shevchenko@intel.com> wrote:
>
> On Wed, Oct 22, 2025 at 05:36:33PM +0200, Bartosz Golaszewski wrote:
> > On Wed, Oct 22, 2025 at 5:25 PM Andy Shevchenko
> > <andriy.shevchenko@intel.com> wrote:
> > > On Wed, Oct 22, 2025 at 03:10:40PM +0200, Bartosz Golaszewski wrote:
>
> ...
>
> > > > +static void string_test_strends(struct kunit *test)
> > > > +{
> > > > +     KUNIT_EXPECT_TRUE(test, strends("foo-bar", "bar"));
> > > > +     KUNIT_EXPECT_TRUE(test, strends("foo-bar", "-bar"));
> > > > +     KUNIT_EXPECT_TRUE(test, strends("foobar", "foobar"));
> > > > +     KUNIT_EXPECT_TRUE(test, strends("foobar", ""));
> > > > +     KUNIT_EXPECT_FALSE(test, strends("bar", "foobar"));
> > > > +     KUNIT_EXPECT_FALSE(test, strends("", "foo"));
> > > > +     KUNIT_EXPECT_FALSE(test, strends("foobar", "ba"));
> > > > +     KUNIT_EXPECT_TRUE(test, strends("", ""));
> > > > +}
> > >
> > > Have you checked the binary file? If you want this to be properly implemented,
> > > generate the suffix. (Actually making the function static inline makes my point
> > > really visible)
> >
> > Andy, this is bikeshedding. This is literally the least important
> > piece of this series. It doesn't matter for the big picture whether
> > this is inlined or not.
>
> It's definitely not a bikeshedding. I try to keep a bit consistency here and
> I don't see the point of bloating a kernel (binary as well) for the function
> that just a couple of lines with simple basic calls.
>
> Also note that with inlined version strlen() for string literals will be
> calculated at _compile-time_! This is clear benefit.
>
> Really, library code is not as simple as dropping something to somewhere...
>

Ok, whatever I'll make it static inline.

Bart
Re: [PATCH v2 01/10] string: provide strends()
Posted by Andy Shevchenko 3 months, 2 weeks ago
On Wed, Oct 22, 2025 at 4:11 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
>
> Implement a function for checking if a string ends with a different
> string and add its kunit test cases.

...

> --- a/include/linux/string.h
> +++ b/include/linux/string.h
> @@ -562,4 +562,6 @@ static inline bool strstarts(const char *str, const char *prefix)
>         return strncmp(str, prefix, strlen(prefix)) == 0;
>  }
>
> +bool strends(const char *str, const char *suffix);

Why not static inline as strstarts()?

-- 
With Best Regards,
Andy Shevchenko
Re: [PATCH v2 01/10] string: provide strends()
Posted by Bartosz Golaszewski 3 months, 2 weeks ago
On Wed, Oct 22, 2025 at 3:34 PM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
>
> On Wed, Oct 22, 2025 at 4:11 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> >
> > Implement a function for checking if a string ends with a different
> > string and add its kunit test cases.
>
> ...
>
> > --- a/include/linux/string.h
> > +++ b/include/linux/string.h
> > @@ -562,4 +562,6 @@ static inline bool strstarts(const char *str, const char *prefix)
> >         return strncmp(str, prefix, strlen(prefix)) == 0;
> >  }
> >
> > +bool strends(const char *str, const char *suffix);
>
> Why not static inline as strstarts()?
>

Because it's not a oneliner.

Bartosz
Re: [PATCH v2 01/10] string: provide strends()
Posted by Andy Shevchenko 3 months, 2 weeks ago
On Wed, Oct 22, 2025 at 03:40:00PM +0200, Bartosz Golaszewski wrote:
> On Wed, Oct 22, 2025 at 3:34 PM Andy Shevchenko
> <andy.shevchenko@gmail.com> wrote:
> > On Wed, Oct 22, 2025 at 4:11 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:

...

> > > static inline bool strstarts(const char *str, const char *prefix)

> > >         return strncmp(str, prefix, strlen(prefix)) == 0;
> > >  }
> > >
> > > +bool strends(const char *str, const char *suffix);
> >
> > Why not static inline as strstarts()?
> 
> Because it's not a oneliner.

So, and how does it answer the question? What is the obstacle here that it may
not be a static inline few-liner?

-- 
With Best Regards,
Andy Shevchenko