On Wed, May 22, 2024 at 9:40 PM Hans de Goede <hdegoede@xxxxxxxxxx> wrote:
> On 5/22/24 7:19 PM, Andy Shevchenko wrote:
> > On Wed, May 22, 2024 at 06:48:07PM +0200, Hans de Goede wrote:
...
> >> + /*
> >> + * str is part of the static_command_line from init/main.c and poking
> >> + * holes in that by writing 0 to it is allowed, as is taking long
> >> + * lasting references to it.
> >> + */
> >> + ts_cmdline_data.acpi_name = strsep(&str, ",");
> >> +
> >> + for (i = 0; i < MAX_CMDLINE_PROPS; i++) {
> >> + name = strsep(&str, ",");
> >> + if (!name)
> >> + break;
> >> +
> >> + /* Replace '=' with 0 and make value point past '=' or NULL */
> >> + value = name;
> >> + strsep(&value, "=");
> >> + if (!value) {
> >> + ts_cmdline_props[i] = PROPERTY_ENTRY_BOOL(name);
> >> + } else if (isdigit(value[0])) {
> >> + ret = kstrtou32(value, 10, &u32val);
> >> + if (ret)
> >> + return ret;
> >> +
> >> + ts_cmdline_props[i] = PROPERTY_ENTRY_U32(name, u32val);
> >> + } else {
> >> + ts_cmdline_props[i] = PROPERTY_ENTRY_STRING(name, value);
> >> + }
> >> + }
> >
> > This reminds me a lot from the next_arg(), can we not reinvent a wheel?
>
> next_arg is meant for parsing different arguments on the kernel cmdline
> split by spaces. It has space as separator hardcoded so it cannot be
> used here.
I believe it's not the first time I hear such an excuse for
duplicating Yet Another (Same) Parser.
If you think you really need another separator, we may patch
next_arg() or add next_arg_any(is_separator_fn *fn) and make
next_arg() to be a wrapper of the other one.
--
With Best Regards,
Andy Shevchenko
