| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <570edde0-cfea-f560-fe83-6077f4f221e5@infradead.org>
Date: Mon, 6 Mar 2023 14:37:39 -0800
From: Randy Dunlap <rdunlap@...radead.org>
To: Thomas Zimmermann <tzimmermann@...e.de>, deller@....de,
paulus@...ba.org, benh@...nel.crashing.org, linux@...linux.org.uk,
pjones@...hat.com, timur@...nel.org, adaplas@...il.com,
s.hauer@...gutronix.de, shawnguo@...nel.org, mbroemme@...mpq.org,
thomas@...ischhofer.net, James.Bottomley@...senPartnership.com,
spock@...too.org, sudipm.mukherjee@...il.com,
teddy.wang@...iconmotion.com, geert+renesas@...der.be,
corbet@....net
Cc: linux-fbdev@...r.kernel.org, dri-devel@...ts.freedesktop.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH 01/99] lib: Add option iterator
Hi,
On 3/6/23 07:58, Thomas Zimmermann 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.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@...e.de>
> ---
> Documentation/core-api/kernel-api.rst | 9 +++
> include/linux/cmdline.h | 29 ++++++++
> lib/Makefile | 2 +-
> lib/cmdline_iter.c | 97 +++++++++++++++++++++++++++
> 4 files changed, 136 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..5d7e648e98a5
> --- /dev/null
> +++ b/include/linux/cmdline.h
> @@ -0,0 +1,29 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +
> +#ifndef LINUX_CMDLINE_H
> +#define LINUX_CMDLINE_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);
> +const 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_.
> + */
> +#define option_iter_next(iter_, opt_) \
> + (((opt_) = option_iter_incr(iter_)) != NULL)
> +
> +#endif
> diff --git a/lib/cmdline_iter.c b/lib/cmdline_iter.c
> new file mode 100644
> index 000000000000..d9371dfea08b
> --- /dev/null
> +++ b/lib/cmdline_iter.c
> @@ -0,0 +1,97 @@
> +// 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. Modules can parse an option string with struct &option_iter and
> + * its helpers. After obtaining the string, initialize and instance of the
an instance
> + * option iterator and loop iver its content as show below.
over
> + *
> + * .. code-block:: c
> + *
> + * const char *options = ...; // provided option string
> + *
> + * struct option_iter iter;
> + * const 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 call to option_iter_init() initializes the iterator instance
> + * from the option string. The while loop walks over the individual
> + * options in the sting and returns each in the second argument. The
> + * returned memory is owned by the iterator instance and callers may
> + * not modify or free it. The call to option_iter_release() frees all
> + * resources of the iterator. This process does not modify the original
> + * option string. If the option string contains an empty option (i.e.,
> + * two commas next to each other), option_iter_next() skips the empty
> + * option automatically.
Is that latter skipping over a ",," automatically something that you have
observed as needed?
I can imagine a driver or module wanting to know that an empty string
was entered (i.e., ",,").
> + */
> +
> +/**
> + * 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
> + *
> + * Returns:
* Return:
matches kernel-doc notation documentation.
> + * The current option string, or NULL if there are no more options.
> + */
> +const 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);
Looks useful. Thanks.
--
~Randy
Powered by blists - more mailing lists