Re: [PATCH RESEND v2 17/32] drm/vkms: Introduce configfs for plane format

[Louis Chauvet]

[...]

> > +		.expected_offset = 1
> > +	}, {
> > +		.data = "-R1111",
> > +		.data_len = 3,
>
> The string is longer than 3 chars, is this intended?

Yes, I wanted to ensure that the algorithm stop at data_len and not \0

> > --- a/drivers/gpu/drm/vkms/vkms_configfs.c
> > +++ b/drivers/gpu/drm/vkms/vkms_configfs.c
>
> > +static ssize_t plane_supported_formats_store(struct config_item *item,
> > +					     const char *page, size_t count)
> > +{
> > +	struct vkms_configfs_plane *plane;
> > +
> > +	plane = plane_item_to_vkms_configfs_plane(item);
> > +	int ret = 0;
> > +	const char *end_page = page + count;
> > +
> > +	scoped_guard(mutex, &plane->dev->lock) {
> > +		while (1) {
> > +			char *tmp;
> > +			char fmt[4] = {' ', ' ', ' ', ' '};
> > +			int len = vkms_configfs_parse_next_format(page, end_page, &tmp);
> > +
> > +			// No fourcc code found
> > +			if (len <= 1)
> > +				break;
> > +
> > +			page = tmp + len;
> > +			memcpy(tmp, &fmt[1], min(len - 1, 4));
> Should this be instead:        fmt   tmp
> ?

memcpy(void *to, const void *from

Make sense yes... My tests were not sufficient :(

I will triple check this before sending the v3

> Also I think it would be good to reject strings longer than 4 chars (len >
> 5), because they cannot br fourccs.

I will add this limitation.

Thanks!

> Otherwise looks good.
>
> BTW, I feel your pain in implementing the parsing!
>
> Luca
>
> --
> Luca Ceresoli, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com