On Thu, 17 Jul 2025 at 16:52, Kees Cook <k...@kernel.org> wrote: > > Add KUnit tests for the seq_buf API to ensure its correctness and > prevent future regressions. > > The tests cover the following functions: > - seq_buf_init() > - DECLARE_SEQ_BUF() > - seq_buf_clear() > - seq_buf_puts() > - seq_buf_putc() > - seq_buf_printf() > - seq_buf_get_buf() > - seq_buf_commit() > > $ tools/testing/kunit/kunit.py run seq_buf > =================== seq_buf (9 subtests) =================== > [PASSED] seq_buf_init_test > [PASSED] seq_buf_declare_test > [PASSED] seq_buf_clear_test > [PASSED] seq_buf_puts_test > [PASSED] seq_buf_puts_overflow_test > [PASSED] seq_buf_putc_test > [PASSED] seq_buf_printf_test > [PASSED] seq_buf_printf_overflow_test > [PASSED] seq_buf_get_buf_commit_test > ===================== [PASSED] seq_buf ===================== > > Signed-off-by: Kees Cook <k...@kernel.org> > --- > I used an LLM to produce this; it did pretty well, but I had to help it > get the Kconfig and make targets in the right places, and I tweaked some > of the edge cases and added a bit more (perhaps redundant) state checking. >
This patch looks good to me. A couple of very minor notes below, but nothing I think is a big issue. I don't have much to say on the LLM use -- it definitely looks better than the last time I tried it. If something like Sasha's suggestion for how to tag LLM contributions[1] goes through, I do think noting what was used could be a useful thing to have. Reviewed-by: David Gow <david...@google.com> -- David [1] https://lore.kernel.org/all/20250725175358.1989323-1-sas...@kernel.org/ > Cc: David Gow <david...@google.com> > Cc: Rae Moar <rm...@google.com> > Cc: Tamir Duberstein <tam...@gmail.com> > Cc: Eric Biggers <ebigg...@kernel.org> > Cc: "Steven Rostedt (Google)" <rost...@goodmis.org> > Cc: Andy Shevchenko <andriy.shevche...@linux.intel.com> > Cc: Petr Mladek <pmla...@suse.com> > Cc: "Matthew Wilcox (Oracle)" <wi...@infradead.org> > --- > lib/Kconfig.debug | 9 ++ > lib/tests/Makefile | 1 + > lib/tests/seq_buf_kunit.c | 205 ++++++++++++++++++++++++++++++++++++++ > 3 files changed, 215 insertions(+) > create mode 100644 lib/tests/seq_buf_kunit.c > > diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug > index cf05bf1df983..048efc3183d5 100644 > --- a/lib/Kconfig.debug > +++ b/lib/Kconfig.debug > @@ -2470,6 +2470,15 @@ config SCANF_KUNIT_TEST > > If unsure, say N. > > +config SEQ_BUF_KUNIT_TEST > + tristate "KUnit test for seq_buf" if !KUNIT_ALL_TESTS > + depends on KUNIT > + default KUNIT_ALL_TESTS > + help > + This builds unit tests for the seq_buf library. > + > + If unsure, say N. > + FYI: Checkpatch complains a bit that this is too short a description. It doesn't bother me much, though: WARNING: please write a help paragraph that fully describes the config symbol with at least 4 lines > config STRING_KUNIT_TEST > tristate "KUnit test string functions at runtime" if !KUNIT_ALL_TESTS > depends on KUNIT > diff --git a/lib/tests/Makefile b/lib/tests/Makefile > index 84b15c986b8c..fa6d728a8b5b 100644 > --- a/lib/tests/Makefile > +++ b/lib/tests/Makefile > @@ -36,6 +36,7 @@ obj-$(CONFIG_OVERFLOW_KUNIT_TEST) += overflow_kunit.o > obj-$(CONFIG_PRINTF_KUNIT_TEST) += printf_kunit.o > obj-$(CONFIG_RANDSTRUCT_KUNIT_TEST) += randstruct_kunit.o > obj-$(CONFIG_SCANF_KUNIT_TEST) += scanf_kunit.o > +obj-$(CONFIG_SEQ_BUF_KUNIT_TEST) += seq_buf_kunit.o > obj-$(CONFIG_SIPHASH_KUNIT_TEST) += siphash_kunit.o > obj-$(CONFIG_SLUB_KUNIT_TEST) += slub_kunit.o > obj-$(CONFIG_TEST_SORT) += test_sort.o > diff --git a/lib/tests/seq_buf_kunit.c b/lib/tests/seq_buf_kunit.c > new file mode 100644 > index 000000000000..74648dbda13f > --- /dev/null > +++ b/lib/tests/seq_buf_kunit.c > @@ -0,0 +1,205 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * KUnit tests for the seq_buf API > + * > + * Copyright (C) 2025, Google LLC. > + */ > + > +#include <kunit/test.h> > +#include <linux/seq_buf.h> > + > +static void seq_buf_init_test(struct kunit *test) > +{ > + char buf[32]; > + struct seq_buf s; > + > + seq_buf_init(&s, buf, sizeof(buf)); > + > + KUNIT_EXPECT_EQ(test, s.size, 32); > + KUNIT_EXPECT_EQ(test, s.len, 0); > + KUNIT_EXPECT_FALSE(test, seq_buf_has_overflowed(&s)); > + KUNIT_EXPECT_EQ(test, seq_buf_buffer_left(&s), 32); > + KUNIT_EXPECT_EQ(test, seq_buf_used(&s), 0); > + KUNIT_EXPECT_STREQ(test, seq_buf_str(&s), ""); > +} > + > +static void seq_buf_declare_test(struct kunit *test) > +{ > + DECLARE_SEQ_BUF(s, 24); > + > + KUNIT_EXPECT_EQ(test, s.size, 24); > + KUNIT_EXPECT_EQ(test, s.len, 0); > + KUNIT_EXPECT_FALSE(test, seq_buf_has_overflowed(&s)); > + KUNIT_EXPECT_EQ(test, seq_buf_buffer_left(&s), 24); > + KUNIT_EXPECT_EQ(test, seq_buf_used(&s), 0); > + KUNIT_EXPECT_STREQ(test, seq_buf_str(&s), ""); > +} > + > +static void seq_buf_clear_test(struct kunit *test) > +{ > + DECLARE_SEQ_BUF(s, 128); > + > + seq_buf_puts(&s, "hello"); > + KUNIT_EXPECT_EQ(test, s.len, 5); > + KUNIT_EXPECT_FALSE(test, seq_buf_has_overflowed(&s)); My gut feeling seeing this was that we should test that seq_buf_clear() also clears the overflow flag, though the separate overflow tests below all do seem to test that, so it's probably redundant. > + KUNIT_EXPECT_STREQ(test, seq_buf_str(&s), "hello"); > + > + seq_buf_clear(&s); > + > + KUNIT_EXPECT_EQ(test, s.len, 0); > + KUNIT_EXPECT_FALSE(test, seq_buf_has_overflowed(&s)); > + KUNIT_EXPECT_STREQ(test, seq_buf_str(&s), ""); > +} > + > +static void seq_buf_puts_test(struct kunit *test) > +{ > + DECLARE_SEQ_BUF(s, 16); > + > + seq_buf_puts(&s, "hello"); > + KUNIT_EXPECT_EQ(test, seq_buf_used(&s), 5); > + KUNIT_EXPECT_FALSE(test, seq_buf_has_overflowed(&s)); > + KUNIT_EXPECT_STREQ(test, seq_buf_str(&s), "hello"); > + > + seq_buf_puts(&s, " world"); > + KUNIT_EXPECT_EQ(test, seq_buf_used(&s), 11); > + KUNIT_EXPECT_FALSE(test, seq_buf_has_overflowed(&s)); > + KUNIT_EXPECT_STREQ(test, seq_buf_str(&s), "hello world"); > +} > + > +static void seq_buf_puts_overflow_test(struct kunit *test) > +{ > + DECLARE_SEQ_BUF(s, 10); > + > + seq_buf_puts(&s, "123456789"); > + KUNIT_EXPECT_FALSE(test, seq_buf_has_overflowed(&s)); > + KUNIT_EXPECT_EQ(test, seq_buf_used(&s), 9); > + > + seq_buf_puts(&s, "0"); > + KUNIT_EXPECT_TRUE(test, seq_buf_has_overflowed(&s)); > + KUNIT_EXPECT_EQ(test, seq_buf_used(&s), 10); > + KUNIT_EXPECT_STREQ(test, seq_buf_str(&s), "123456789"); > + > + seq_buf_clear(&s); > + KUNIT_EXPECT_EQ(test, s.len, 0); > + KUNIT_EXPECT_FALSE(test, seq_buf_has_overflowed(&s)); > + KUNIT_EXPECT_STREQ(test, seq_buf_str(&s), ""); > +} > + > +static void seq_buf_putc_test(struct kunit *test) > +{ > + DECLARE_SEQ_BUF(s, 4); > + > + seq_buf_putc(&s, 'a'); > + seq_buf_putc(&s, 'b'); > + seq_buf_putc(&s, 'c'); > + > + KUNIT_EXPECT_EQ(test, seq_buf_used(&s), 3); > + KUNIT_EXPECT_FALSE(test, seq_buf_has_overflowed(&s)); > + KUNIT_EXPECT_STREQ(test, seq_buf_str(&s), "abc"); > + > + seq_buf_putc(&s, 'd'); > + KUNIT_EXPECT_EQ(test, seq_buf_used(&s), 4); > + KUNIT_EXPECT_FALSE(test, seq_buf_has_overflowed(&s)); > + KUNIT_EXPECT_STREQ(test, seq_buf_str(&s), "abc"); > + > + seq_buf_putc(&s, 'e'); > + KUNIT_EXPECT_EQ(test, seq_buf_used(&s), 4); > + KUNIT_EXPECT_TRUE(test, seq_buf_has_overflowed(&s)); > + KUNIT_EXPECT_STREQ(test, seq_buf_str(&s), "abc"); > + > + seq_buf_clear(&s); > + KUNIT_EXPECT_EQ(test, s.len, 0); > + KUNIT_EXPECT_FALSE(test, seq_buf_has_overflowed(&s)); > + KUNIT_EXPECT_STREQ(test, seq_buf_str(&s), ""); > +} > + > +static void seq_buf_printf_test(struct kunit *test) > +{ > + DECLARE_SEQ_BUF(s, 32); > + > + seq_buf_printf(&s, "hello %s", "world"); > + KUNIT_EXPECT_EQ(test, seq_buf_used(&s), 11); > + KUNIT_EXPECT_FALSE(test, seq_buf_has_overflowed(&s)); > + KUNIT_EXPECT_STREQ(test, seq_buf_str(&s), "hello world"); > + > + seq_buf_printf(&s, " %d", 123); > + KUNIT_EXPECT_EQ(test, seq_buf_used(&s), 15); > + KUNIT_EXPECT_FALSE(test, seq_buf_has_overflowed(&s)); > + KUNIT_EXPECT_STREQ(test, seq_buf_str(&s), "hello world 123"); > +} > + > +static void seq_buf_printf_overflow_test(struct kunit *test) > +{ > + DECLARE_SEQ_BUF(s, 16); > + > + seq_buf_printf(&s, "%lu", 1234567890UL); > + KUNIT_EXPECT_FALSE(test, seq_buf_has_overflowed(&s)); > + KUNIT_EXPECT_EQ(test, seq_buf_used(&s), 10); > + KUNIT_EXPECT_STREQ(test, seq_buf_str(&s), "1234567890"); > + > + seq_buf_printf(&s, "%s", "abcdefghij"); > + KUNIT_EXPECT_TRUE(test, seq_buf_has_overflowed(&s)); > + KUNIT_EXPECT_EQ(test, seq_buf_used(&s), 16); > + KUNIT_EXPECT_STREQ(test, seq_buf_str(&s), "1234567890abcde"); > + > + seq_buf_clear(&s); > + KUNIT_EXPECT_EQ(test, s.len, 0); > + KUNIT_EXPECT_FALSE(test, seq_buf_has_overflowed(&s)); > + KUNIT_EXPECT_STREQ(test, seq_buf_str(&s), ""); > +} > + > +static void seq_buf_get_buf_commit_test(struct kunit *test) > +{ > + DECLARE_SEQ_BUF(s, 16); > + char *buf; > + size_t len; > + > + len = seq_buf_get_buf(&s, &buf); > + KUNIT_EXPECT_EQ(test, len, 16); > + KUNIT_EXPECT_PTR_NE(test, buf, NULL); > + > + memcpy(buf, "hello", 5); > + seq_buf_commit(&s, 5); > + > + KUNIT_EXPECT_EQ(test, seq_buf_used(&s), 5); > + KUNIT_EXPECT_FALSE(test, seq_buf_has_overflowed(&s)); > + KUNIT_EXPECT_STREQ(test, seq_buf_str(&s), "hello"); > + > + len = seq_buf_get_buf(&s, &buf); > + KUNIT_EXPECT_EQ(test, len, 11); > + KUNIT_EXPECT_PTR_NE(test, buf, NULL); > + > + memcpy(buf, " worlds!", 8); > + seq_buf_commit(&s, 6); > + > + KUNIT_EXPECT_EQ(test, seq_buf_used(&s), 11); > + KUNIT_EXPECT_FALSE(test, seq_buf_has_overflowed(&s)); > + KUNIT_EXPECT_STREQ(test, seq_buf_str(&s), "hello world"); > + > + len = seq_buf_get_buf(&s, &buf); > + KUNIT_EXPECT_EQ(test, len, 5); > + KUNIT_EXPECT_PTR_NE(test, buf, NULL); > + > + seq_buf_commit(&s, -1); > + KUNIT_EXPECT_TRUE(test, seq_buf_has_overflowed(&s)); > +} > + > +static struct kunit_case seq_buf_test_cases[] = { > + KUNIT_CASE(seq_buf_init_test), > + KUNIT_CASE(seq_buf_declare_test), > + KUNIT_CASE(seq_buf_clear_test), > + KUNIT_CASE(seq_buf_puts_test), > + KUNIT_CASE(seq_buf_puts_overflow_test), > + KUNIT_CASE(seq_buf_putc_test), > + KUNIT_CASE(seq_buf_printf_test), > + KUNIT_CASE(seq_buf_printf_overflow_test), > + KUNIT_CASE(seq_buf_get_buf_commit_test), > + {} > +}; > + > +static struct kunit_suite seq_buf_test_suite = { > + .name = "seq_buf", > + .test_cases = seq_buf_test_cases, > +}; > + > +kunit_test_suite(seq_buf_test_suite); > -- > 2.34.1 >
smime.p7s
Description: S/MIME Cryptographic Signature
