[01/99] 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.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.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
  

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@suse.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.
  

Hi

Am 06.03.23 um 23:37 schrieb Randy Dunlap:
[...]
>> + *
>> + * 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?

It's not strictly needed for correctness, but many of those fbdev 
drivers contain code to do that. Like this one:

 
https://elixir.bootlin.com/linux/v6.2/source/drivers/video/fbdev/vesafb.c#L217

So doing it in the _incr() helper seems useful

> I can imagine a driver or module wanting to know that an empty string
> was entered (i.e., ",,").

I only looked at fbdev drivers, but none of them cared about empty 
strings. They all have named options and/or key-value pairs.

> 
>> + */
>> +
>> +/**
>> + * 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.

Thanks.

Best regards
Thomas

> 

-- 
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