Re: [PATCH 3/5] drivers: usb: host: efi: add efi io driver

On 2026-08-12 18:36, Fabian Pflug wrote:
> +static int efi_get_usb_string(struct efi_usb_io_protocol *protocol, u16 lang_id,
> +			      int index, char *buf, size_t size)
> +{
> +	char *efi_name;
> +	efi_status_t efiret;
> +	unsigned int u, idx;
> +
> +	memset(buf, 0, size);
> +
> +	if (!index)
> +		return 0;
> +
> +	efiret = protocol->get_string_descriptor(protocol, lang_id, index,
> +						 &efi_name);

Should this be declared with wchar_t * as last argument? That's how it's
declared in EDK2 and the below code easier to follow.

> +	if (EFI_ERROR(efiret))
> +		return -efi_errno(efiret);
> +
> +	size--; /* leave room for trailing NULL char in output buffer */
> +	for (idx = 0, u = 2;; u += 2) {

It's surprising to have u = 2 here. What's in the first character?

> +		if (idx >= size)
> +			break;
> +		if (efi_name[u + 1]) /* high byte */
> +			buf[idx++] = '?'; /* non-ASCII character */
> +		else if (efi_name[u])
> +			buf[idx++] = efi_name[u];
> +		else
> +			break;
> +	}
> +	buf[idx] = 0;
> +
> +	return 0;
> +}
> +

...

> +
> +	efiret = priv->protocol->get_supported_languages(priv->protocol,
> +							 &lang_ids, &num_langs);
> +	if (EFI_ERROR(efiret)) {
> +		err = -efi_errno(efiret);
> +		goto out_err;
> +	}
> +
> +	num_langs = num_langs / sizeof(u16);

Pass a table_size variable to get_supported_languages() and calculate
num_langs_from it, makes it easier to follow without looking at the
prototype.

> +	if (num_langs == 0) {
> +		err = -EPROTO;
> +		goto out_err;
> +	}
> +

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |