Re: [PATCH v5 05/10] gpiolib: Add gpio_property_name_length()

[Andy Shevchenko <andriy.shevchenko@linux.intel.com>]

On Thu, Aug 22, 2024 at 05:19:58PM +0800, Chen-Yu Tsai wrote:
> The I2C device tree component prober needs to get and toggle GPIO lines
> for the components it intends to probe. These components may not use the
> same name for their GPIO lines, so the prober must go through the device
> tree, check each property to see it is a GPIO property, and get the GPIO
> line.
> 
> Instead of duplicating the GPIO suffixes, or exporting them to the
> prober to do pattern matching, simply add and export a new function that
> does the pattern matching and returns the length of the GPIO name. The
> caller can then use that to copy out the name if it needs to.

...

> +/**
> + * gpio_property_name_length - Returns the GPIO name length from a property name
> + * @str:	string to check

It's property name, so, I would name this 'propname'.

> + * This function checks if the given name matches the GPIO property patterns, and
> + * returns the length of the name of the GPIO. The pattern is "*-<GPIO suffix>"
> + * or just "<GPIO suffix>".
> + *
> + * Returns:
> + * The length of the string before '-' if it matches "*-<GPIO suffix>", or

What about "x-y-gpios"? It's unclear what will be the behaviour.

> + * 0 if no name part, just the suffix, or
> + * -EINVAL if the string doesn't match the pattern.
> + */
> +int gpio_property_name_length(const char *str)

gpio_get_... ?

> +{
> +	size_t len;
> +
> +	len = strlen(str);

If it has a thousands characters...?

> +	/* string need to be at minimum len(gpio) */
> +	if (len < 4)
> +		return -EINVAL;

Do we really need it here? See below as well.

> +	/* Check for no-name case: "gpio" / "gpios" */
> +	for (const char *const *p = gpio_suffixes; *p; p++)
> +		if (!strcmp(str, *p))
> +			return 0;

> +	for (size_t i = len - 4; i > 0; i--) {
> +		/* find right-most '-' and check if remainder matches suffix */
> +		if (str[i] != '-')
> +			continue;
> +
> +		for (const char *const *p = gpio_suffixes; *p; p++)
> +			if (!strcmp(str + i + 1, *p))
> +				return i;
> +
> +		return -EINVAL;
> +	}

This can be combined with the above

	for (const char *const *p = gpio_suffixes; *p; p++) {
		/*
		 * Find right-most '-' and check if remainder matches suffix.
		 * If no separator found, check for no-name cases.
		 */
		dash = strrchr(propname, '-');
		if (!strcmp(dash ? dash + 1 : propname, *p))
			return i;
	}

> +	return -EINVAL;
> +}

-- 
With Best Regards,
Andy Shevchenko