2023-03-09 16:02:42

by Thomas Zimmermann

[permalink] [raw]
Subject: [PATCH v2 001/101] lib: Add option iterator

Add struct option_iter and helpers that walk over individual options
of an option string. Add documentation.

Kernel parameters often have the format of

param=opt1,opt2:val,opt3

where the option string contains a number of comma-separated options.
Drivers usually use strsep() in a loop to extract individual options
from the string. Each call to strsep() modifies the given string, so
callers have to duplicate kernel parameters that are to be parsed
multiple times.

The new struct option_iter and its helpers wrap this code behind a
clean interface. Drivers can iterate over the options without having
to know the details of the option-string format. The iterator handles
string memory internally without modifying the original options.

v2:
* improve documentation (Randy)
* implement option_iter_next() as function
* allow modification of the returned options

Signed-off-by: Thomas Zimmermann <[email protected]>
---
Documentation/core-api/kernel-api.rst | 9 +++
include/linux/cmdline.h | 36 +++++++++
lib/Makefile | 2 +-
lib/cmdline_iter.c | 109 ++++++++++++++++++++++++++
4 files changed, 155 insertions(+), 1 deletion(-)
create mode 100644 include/linux/cmdline.h
create mode 100644 lib/cmdline_iter.c

diff --git a/Documentation/core-api/kernel-api.rst b/Documentation/core-api/kernel-api.rst
index 62f961610773..cdc7ba8decf9 100644
--- a/Documentation/core-api/kernel-api.rst
+++ b/Documentation/core-api/kernel-api.rst
@@ -93,9 +93,18 @@ Bitmap Operations
Command-line Parsing
--------------------

+.. kernel-doc:: lib/cmdline_iter.c
+ :doc: overview
+
.. kernel-doc:: lib/cmdline.c
:export:

+.. kernel-doc:: lib/cmdline_iter.c
+ :export:
+
+.. kernel-doc:: include/linux/cmdline.h
+ :internal:
+
Sorting
-------

diff --git a/include/linux/cmdline.h b/include/linux/cmdline.h
new file mode 100644
index 000000000000..e9e0eb68e176
--- /dev/null
+++ b/include/linux/cmdline.h
@@ -0,0 +1,36 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+#ifndef LINUX_CMDLINE_H
+#define LINUX_CMDLINE_H
+
+#include <linux/types.h>
+
+/**
+ * struct option_iter - Iterates over string of kernel or module options
+ */
+struct option_iter {
+ char *optbuf;
+ char *next_opt;
+};
+
+void option_iter_init(struct option_iter *iter, const char *options);
+void option_iter_release(struct option_iter *iter);
+char *option_iter_incr(struct option_iter *iter);
+
+/**
+ * option_iter_next - Loop condition to move over options
+ * @iter: the iterator
+ * @opt: the name of the option variable
+ *
+ * Iterates over option strings as part of a while loop and
+ * stores the current option in @opt.
+ *
+ * Return:
+ * True to continue the loop, or false if no further option has been found.
+ */
+static inline bool option_iter_next(struct option_iter *iter, char **opt)
+{
+ return ((*opt = option_iter_incr(iter)) != NULL);
+}
+
+#endif
diff --git a/lib/Makefile b/lib/Makefile
index 4d9461bfea42..829ea6647d7a 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -27,7 +27,7 @@ KASAN_SANITIZE_string.o := n
CFLAGS_string.o += -fno-stack-protector
endif

-lib-y := ctype.o string.o vsprintf.o cmdline.o \
+lib-y := ctype.o string.o vsprintf.o cmdline.o cmdline_iter.o \
rbtree.o radix-tree.o timerqueue.o xarray.o \
maple_tree.o idr.o extable.o irq_regs.o argv_split.o \
flex_proportions.o ratelimit.o show_mem.o \
diff --git a/lib/cmdline_iter.c b/lib/cmdline_iter.c
new file mode 100644
index 000000000000..3e53aa8d8a3d
--- /dev/null
+++ b/lib/cmdline_iter.c
@@ -0,0 +1,109 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+#include <linux/cmdline.h>
+#include <linux/export.h>
+#include <linux/slab.h>
+
+/**
+ * DOC: overview
+ *
+ * A kernel parameter's option string can contain multiple comma-separated
+ * options. Struct &option_iter and its helpers parse the string and return
+ * the individual substrings.
+ *
+ * After obtaining the string from the kernel, initialize an instance of the
+ * option iterator and loop over its content as show below.
+ *
+ * .. code-block:: c
+ *
+ * const char *options = ...; // kernel-provided option string
+ *
+ * struct option_iter iter;
+ * char *opt;
+ *
+ * option_iter_init(&iter, options);
+ *
+ * while (option_iter_next(&iter, &opt)) {
+ * if (!strcmp(opt, "foo"))
+ * ...
+ * else (strcmp(opt, "bar"))
+ * ...
+ * else
+ * pr_warn("unknown option %s\n", opt);
+ * }
+ *
+ * option_iter_release(&iter);
+ *
+ * The functions option_iter_init() and option_iter_release() enclose
+ * all code that uses struct &option_iter. The call to option_iter_init()
+ * initializes the iterator instance from the option string and sets it
+ * up to return the first contained substring. The iterator owns a copy
+ * of the option string, which is later freed by option_iter_release().
+ * The state of an instance of struct &option_iter is undefined after the
+ * release function has returned.
+ *
+ * The loop walks over the individual options while option_iter_next()
+ * succeeds. In this case, the second argument returns a pointer to the
+ * contained substring. If the option string contains an empty option
+ * (i.e., two commas next to each other), option_iter_next() skips the
+ * empty option automatically.
+ *
+ * The parsing process does not modify the original option string that
+ * was used to initialize the iterator. The memory returned by
+ * option_iter_next() is owned by the iterator instance. Callers are
+ * allowed to modify its content but may not free it. References to the
+ * returned string become stale after option_iter_release() has returned.
+ */
+
+/**
+ * option_iter_init - Initializes an option iterator
+ * @iter: the iterator to initialize
+ * @options: the options string
+ */
+void option_iter_init(struct option_iter *iter, const char *options)
+{
+ if (options && *options)
+ iter->optbuf = kstrdup(options, GFP_KERNEL); // can be NULL
+ else
+ iter->optbuf = NULL;
+ iter->next_opt = iter->optbuf;
+}
+EXPORT_SYMBOL(option_iter_init);
+
+/**
+ * option_iter_release - Releases an option iterator's resources
+ * @iter: the iterator
+ */
+void option_iter_release(struct option_iter *iter)
+{
+ kfree(iter->optbuf);
+ iter->next_opt = NULL;
+}
+EXPORT_SYMBOL(option_iter_release);
+
+/**
+ * option_iter_incr - Return current option and advance to the next
+ * @iter: the iterator
+ *
+ * Return:
+ * The current option string, or NULL if there are no more options.
+ */
+char *option_iter_incr(struct option_iter *iter)
+{
+ char *opt;
+
+ if (!iter->next_opt) { // can be OK if kstrdup failed
+ if (iter->optbuf) // iter has already been released; logic error
+ pr_err("Incrementing option iterator without string\n");
+ return NULL;
+ }
+
+ do {
+ opt = strsep(&iter->next_opt, ",");
+ if (!opt)
+ return NULL;
+ } while (!*opt); // found empty option string, try next
+
+ return opt;
+}
+EXPORT_SYMBOL(option_iter_incr);
--
2.39.2



2023-03-10 08:29:42

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [PATCH v2 001/101] lib: Add option iterator

Hi Thomas,

On Thu, Mar 9, 2023 at 5:02 PM Thomas Zimmermann <[email protected]> wrote:
> Add struct option_iter and helpers that walk over individual options
> of an option string. Add documentation.
>
> Kernel parameters often have the format of
>
> param=opt1,opt2:val,opt3
>
> where the option string contains a number of comma-separated options.
> Drivers usually use strsep() in a loop to extract individual options
> from the string. Each call to strsep() modifies the given string, so
> callers have to duplicate kernel parameters that are to be parsed
> multiple times.
>
> The new struct option_iter and its helpers wrap this code behind a
> clean interface. Drivers can iterate over the options without having
> to know the details of the option-string format. The iterator handles
> string memory internally without modifying the original options.
>
> v2:
> * improve documentation (Randy)
> * implement option_iter_next() as function
> * allow modification of the returned options
>
> Signed-off-by: Thomas Zimmermann <[email protected]>

Thanks for your patch!

I> ---
> Documentation/core-api/kernel-api.rst | 9 +++
> include/linux/cmdline.h | 36 +++++++++
> lib/Makefile | 2 +-
> lib/cmdline_iter.c | 109 ++++++++++++++++++++++++++
> 4 files changed, 155 insertions(+), 1 deletion(-)
> create mode 100644 include/linux/cmdline.h
> create mode 100644 lib/cmdline_iter.c

Just wondering, is there any code that can be shared with/reused from
the existing lib/cmdline.c?

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

2023-03-10 12:39:01

by Thomas Zimmermann

[permalink] [raw]
Subject: Re: [PATCH v2 001/101] lib: Add option iterator

Hi

Am 10.03.23 um 09:21 schrieb Geert Uytterhoeven:
> Hi Thomas,
>
> On Thu, Mar 9, 2023 at 5:02 PM Thomas Zimmermann <[email protected]> wrote:
>> Add struct option_iter and helpers that walk over individual options
>> of an option string. Add documentation.
>>
>> Kernel parameters often have the format of
>>
>> param=opt1,opt2:val,opt3
>>
>> where the option string contains a number of comma-separated options.
>> Drivers usually use strsep() in a loop to extract individual options
>> from the string. Each call to strsep() modifies the given string, so
>> callers have to duplicate kernel parameters that are to be parsed
>> multiple times.
>>
>> The new struct option_iter and its helpers wrap this code behind a
>> clean interface. Drivers can iterate over the options without having
>> to know the details of the option-string format. The iterator handles
>> string memory internally without modifying the original options.
>>
>> v2:
>> * improve documentation (Randy)
>> * implement option_iter_next() as function
>> * allow modification of the returned options
>>
>> Signed-off-by: Thomas Zimmermann <[email protected]>
>
> Thanks for your patch!
>
> I> ---
>> Documentation/core-api/kernel-api.rst | 9 +++
>> include/linux/cmdline.h | 36 +++++++++
>> lib/Makefile | 2 +-
>> lib/cmdline_iter.c | 109 ++++++++++++++++++++++++++
>> 4 files changed, 155 insertions(+), 1 deletion(-)
>> create mode 100644 include/linux/cmdline.h
>> create mode 100644 lib/cmdline_iter.c
>
> Just wondering, is there any code that can be shared with/reused from
> the existing lib/cmdline.c?

AFAIU, cmdline.o is linked into early-boot code, which prevents it from
using dynamic memory allocation, such as kstrdup() and kfree(). The
linker warns about this:

LD arch/x86/boot/compressed/vmlinux
ld: arch/x86/boot/compressed/kaslr.o: in function `get_option':
kaslr.c:(.text+0x532): undefined reference to `kstrdup'
ld: arch/x86/boot/compressed/vmlinux: hidden symbol `kstrdup' isn't
defined
ld: final link failed: bad value

So the iterator is in a separate source file.

The code in cmdline.c mostly deals with parsing individual values or
key-value pairs. Nothing moves over the complete string and returns
options one by one. There's not much to share AFAICT.

Best regards
Thomas


>
> Gr{oetje,eeting}s,
>
> Geert
>

--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev


Attachments:
OpenPGP_signature (840.00 B)
OpenPGP digital signature