riscv: supports_extension() broken for long isa strings

[Conor Dooley]

Apologies for the delay replying here.

On Thu, Feb 22, 2024 at 01:36:41PM +0100, Heinrich Schuchardt wrote:
> On 21.02.24 18:59, Conor Dooley wrote:
> > I mentioned this last night to Heinrich on IRC, supports_extension() is
> > broken for ISA strings longer than 32 characters. M-Mode U-Boot doesn't
> > parse a devicetree, so this doesn't apply there, but for S-mode
> > supports_extension() looks like:
> > 
> > static inline bool supports_extension(char ext)
> > {
> > 
> > 	struct udevice *dev;
> > 	char desc[32];
> > 	int i;
> > 
> > 	uclass_find_first_device(UCLASS_CPU, &dev);
> > 	if (!dev) {
> > 		debug("unable to find the RISC-V cpu device\n");
> > 		return false;
> > 	}
> > 	if (!cpu_get_desc(dev, desc, sizeof(desc))) {
> > 		/*
> > 		 * skip the first 4 characters (rv32|rv64) and
> > 		 * check until underscore
> > 		 */
> > 		for (i = 4; i < sizeof(desc); i++) {
> > 			if (desc[i] == '_' || desc[i] == '\0')
> > 				break;
> > 			if (desc[i] == ext)
> > 				return true;
> > 		}
> > 	}
> > 
> > 	return false;
> > }
> > 
> > cpu_get_desc is implemented by riscv_cpu_get_desc():
> > static int riscv_cpu_get_desc(const struct udevice *dev, char *buf, int size)
> 
> Thanks Conor for reporting the issue. We should change all cpu_get_desc
> implementations to:
> 
> int riscv_cpu_get_desc(const struct udevice *dev, char *buf, size_t *size)
> {
> 	size_t old_size = *size;
> 
> 	*size = snprintf(buf, *size, "%s", info) + 1;
> 	if (*size > old_size)
> 		return -ENOSPC;
> 	return 0;
> }
> 
> With this change
> 
> size = 0;
> cpu_get_desc(dev, desc, &size);
> 
> can be used to get the size of the information before allocating a buffer.
> 
> desc = malloc(size);
> cpu_get_desc(dev, desc, size);

That seems overcomplicated to me, if we are just looking to fix this in
the short term. Could we just write to the provided buffer up to a max
of size and report ENOSPC if it does not fit?
That way the calling code in 90% of cases does not need to change and
the supports_extension() code, which does not care about anything other
than single-letter extensions that have to be in the first 32 characters
of the string, can opt to ignore the ENOSPC?

Cheers,
Conor.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 228 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20240304/1a93d8db/attachment.sig>