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

[Louis Chauvet <louis.chauvet@bootlin.com>]

[...]

>> +		.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