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

Louis Chauvet
2025-12-19T17:28:38Z

[...]

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